From 0b84609bbd73f26e6125c42f841e2b54fc7876a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 20 May 2021 18:42:57 +0100 Subject: [PATCH 1/7] gitlab: explicitly reference the upstream registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since c8e6793903 ("containers.yml: build with docker.py tooling") we don't need to manually pull stuff from the upstream repository. Just set the -r field to explicitly use that rather than the current registry. Signed-off-by: Alex Bennée Tested-by: Philippe Mathieu-Daudé Reviewed-by: Willian Rampazzo Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210520174303.12310-3-alex.bennee@linaro.org> --- .gitlab-ci.d/containers.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml index 765408ae27..3fb3c14f06 100644 --- a/.gitlab-ci.d/containers.yml +++ b/.gitlab-ci.d/containers.yml @@ -12,10 +12,9 @@ script: - echo "TAG:$TAG" - echo "COMMON_TAG:$COMMON_TAG" - - docker pull "$TAG" || docker pull "$COMMON_TAG" || true - ./tests/docker/docker.py --engine docker build -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker" - -r $CI_REGISTRY_IMAGE + -r $CI_REGISTRY/qemu-project/qemu - docker tag "qemu/$NAME" "$TAG" - docker push "$TAG" after_script: From 1aab5f053655882fa84df203df5f57599829cd48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 20 May 2021 18:42:58 +0100 Subject: [PATCH 2/7] gitlab: add special rule for the hexagon container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hexagon container is always manually built but of course not everyone will be building it themselves and pushing to their registries. We still need to create a "local" registry copy for the actual gitlab tests to run. We don't build it in this case, just pull it across from the upstream registry. We disable this rule from running on the qemu-project itself so it doesn't accidentally wipe out our master copy. Fixes: 910c40ee94 ("gitlab: add build-user-hexagon test") Signed-off-by: Alex Bennée Tested-by: Cornelia Huck Reviewed-by: Willian Rampazzo Reviewed-by: Philippe Mathieu-Daudé Cc: Cornelia Huck Message-Id: <20210520174303.12310-4-alex.bennee@linaro.org> --- .gitlab-ci.d/containers.yml | 28 ++++++++++++++++++++++++++++ .gitlab-ci.yml | 5 ++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml index 3fb3c14f06..7b7ca3790d 100644 --- a/.gitlab-ci.d/containers.yml +++ b/.gitlab-ci.d/containers.yml @@ -101,6 +101,34 @@ armhf-debian-cross-container: variables: NAME: debian-armhf-cross +# We never want to build hexagon in the CI system and by default we +# always want to refer to the master registry where it lives. +hexagon-cross-container: + image: docker:stable + stage: containers + rules: + - if: '$CI_PROJECT_NAMESPACE == "qemu-project"' + when: never + - when: always + variables: + NAME: debian-hexagon-cross + GIT_DEPTH: 1 + services: + - docker:dind + before_script: + - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest" + - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest" + - docker info + - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" + script: + - echo "TAG:$TAG" + - echo "COMMON_TAG:$COMMON_TAG" + - docker pull $COMMON_TAG + - docker tag $COMMON_TAG $TAG + - docker push "$TAG" + after_script: + - docker logout + hppa-debian-cross-container: extends: .container_job_template stage: containers-layer2 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f718b61fa7..6a0d311cf4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -417,10 +417,13 @@ build-user-static: MAKE_CHECK_ARGS: check-tcg # Because the hexagon cross-compiler takes so long to build we don't rely -# on the CI system to build it and hence this job has no dependency +# on the CI system to build it and hence this job has an optional dependency # declared. The image is manually uploaded. build-user-hexagon: extends: .native_build_job_template + needs: + job: hexagon-cross-container + optional: true variables: IMAGE: debian-hexagon-cross TARGETS: hexagon-linux-user From 305bea066d66cedef0291841f9c6ee3690df3631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 20 May 2021 18:42:59 +0100 Subject: [PATCH 3/7] gdbstub: Constify GdbCmdParseEntry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20210505170055.1415360-3-philmd@redhat.com> Message-Id: <20210520174303.12310-5-alex.bennee@linaro.org> --- gdbstub.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 9103ffc902..83d47c6732 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1981,7 +1981,7 @@ static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx) exit(0); } -static GdbCmdParseEntry gdb_v_commands_table[] = { +static const GdbCmdParseEntry gdb_v_commands_table[] = { /* Order is important if has same prefix */ { .handler = handle_v_cont_query, @@ -2324,7 +2324,7 @@ static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx) } #endif -static GdbCmdParseEntry gdb_gen_query_set_common_table[] = { +static const GdbCmdParseEntry gdb_gen_query_set_common_table[] = { /* Order is important if has same prefix */ { .handler = handle_query_qemu_sstepbits, @@ -2342,7 +2342,7 @@ static GdbCmdParseEntry gdb_gen_query_set_common_table[] = { }, }; -static GdbCmdParseEntry gdb_gen_query_table[] = { +static const GdbCmdParseEntry gdb_gen_query_table[] = { { .handler = handle_query_curr_tid, .cmd = "C", @@ -2420,7 +2420,7 @@ static GdbCmdParseEntry gdb_gen_query_table[] = { #endif }; -static GdbCmdParseEntry gdb_gen_set_table[] = { +static const GdbCmdParseEntry gdb_gen_set_table[] = { /* Order is important if has same prefix */ { .handler = handle_set_qemu_sstep, From 26a16181fafa0f6a4646c9626afda8b7f663c91b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 25 May 2021 09:24:14 +0100 Subject: [PATCH 4/7] gdbstub: Replace GdbCmdContext with plain g_array() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of jumping through hoops let glib deal with both tracking the number of elements and auto freeing the memory once we are done. This allows is to drop the usage of ALLOCA(3) which the man-page mentions its "use is discouraged". Signed-off-by: Alex Bennée Message-Id: <20210520174303.12310-6-alex.bennee@linaro.org> --- gdbstub.c | 322 ++++++++++++++++++++++++++---------------------------- 1 file changed, 154 insertions(+), 168 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 83d47c6732..84ce770a04 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1338,6 +1338,8 @@ typedef union GdbCmdVariant { } thread_id; } GdbCmdVariant; +#define get_param(p, i) (&g_array_index(p, GdbCmdVariant, i)) + static const char *cmd_next_param(const char *param, const char delimiter) { static const char all_delimiters[] = ",;:="; @@ -1363,55 +1365,52 @@ static const char *cmd_next_param(const char *param, const char delimiter) } static int cmd_parse_params(const char *data, const char *schema, - GdbCmdVariant *params, int *num_params) + GArray *params) { - int curr_param; const char *curr_schema, *curr_data; - *num_params = 0; - - if (!schema) { - return 0; - } + g_assert(schema); + g_assert(params->len == 0); curr_schema = schema; - curr_param = 0; curr_data = data; while (curr_schema[0] && curr_schema[1] && *curr_data) { + GdbCmdVariant this_param; + switch (curr_schema[0]) { case 'l': if (qemu_strtoul(curr_data, &curr_data, 16, - ¶ms[curr_param].val_ul)) { + &this_param.val_ul)) { return -EINVAL; } - curr_param++; curr_data = cmd_next_param(curr_data, curr_schema[1]); + g_array_append_val(params, this_param); break; case 'L': if (qemu_strtou64(curr_data, &curr_data, 16, - (uint64_t *)¶ms[curr_param].val_ull)) { + (uint64_t *)&this_param.val_ull)) { return -EINVAL; } - curr_param++; curr_data = cmd_next_param(curr_data, curr_schema[1]); + g_array_append_val(params, this_param); break; case 's': - params[curr_param].data = curr_data; - curr_param++; + this_param.data = curr_data; curr_data = cmd_next_param(curr_data, curr_schema[1]); + g_array_append_val(params, this_param); break; case 'o': - params[curr_param].opcode = *(uint8_t *)curr_data; - curr_param++; + this_param.opcode = *(uint8_t *)curr_data; curr_data = cmd_next_param(curr_data, curr_schema[1]); + g_array_append_val(params, this_param); break; case 't': - params[curr_param].thread_id.kind = + this_param.thread_id.kind = read_thread_id(curr_data, &curr_data, - ¶ms[curr_param].thread_id.pid, - ¶ms[curr_param].thread_id.tid); - curr_param++; + &this_param.thread_id.pid, + &this_param.thread_id.tid); curr_data = cmd_next_param(curr_data, curr_schema[1]); + g_array_append_val(params, this_param); break; case '?': curr_data = cmd_next_param(curr_data, curr_schema[1]); @@ -1422,16 +1421,10 @@ static int cmd_parse_params(const char *data, const char *schema, curr_schema += 2; } - *num_params = curr_param; return 0; } -typedef struct GdbCmdContext { - GdbCmdVariant *params; - int num_params; -} GdbCmdContext; - -typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx); +typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx); /* * cmd_startswith -> cmd is compared using startswith @@ -1471,8 +1464,8 @@ static inline int startswith(const char *string, const char *pattern) static int process_string_cmd(void *user_ctx, const char *data, const GdbCmdParseEntry *cmds, int num_cmds) { - int i, schema_len, max_num_params = 0; - GdbCmdContext gdb_ctx; + int i; + g_autoptr(GArray) params = g_array_new(false, true, sizeof(GdbCmdVariant)); if (!cmds) { return -1; @@ -1488,24 +1481,13 @@ static int process_string_cmd(void *user_ctx, const char *data, } if (cmd->schema) { - schema_len = strlen(cmd->schema); - if (schema_len % 2) { - return -2; + if (cmd_parse_params(&data[strlen(cmd->cmd)], + cmd->schema, params)) { + return -1; } - - max_num_params = schema_len / 2; } - gdb_ctx.params = - (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params); - memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params); - - if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema, - gdb_ctx.params, &gdb_ctx.num_params)) { - return -1; - } - - cmd->handler(&gdb_ctx, user_ctx); + cmd->handler(params, user_ctx); return 0; } @@ -1528,18 +1510,18 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd) } } -static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_detach(GArray *params, void *user_ctx) { GDBProcess *process; uint32_t pid = 1; if (gdbserver_state.multiprocess) { - if (!gdb_ctx->num_params) { + if (!params->len) { put_packet("E22"); return; } - pid = gdb_ctx->params[0].val_ul; + pid = get_param(params, 0)->val_ul; } process = gdb_get_process(pid); @@ -1562,22 +1544,22 @@ static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet("OK"); } -static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_thread_alive(GArray *params, void *user_ctx) { CPUState *cpu; - if (!gdb_ctx->num_params) { + if (!params->len) { put_packet("E22"); return; } - if (gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) { + if (get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) { put_packet("E22"); return; } - cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid, - gdb_ctx->params[0].thread_id.tid); + cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid, + get_param(params, 0)->thread_id.tid); if (!cpu) { put_packet("E22"); return; @@ -1586,17 +1568,17 @@ static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet("OK"); } -static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_continue(GArray *params, void *user_ctx) { - if (gdb_ctx->num_params) { - gdb_set_cpu_pc(gdb_ctx->params[0].val_ull); + if (params->len) { + gdb_set_cpu_pc(get_param(params, 0)->val_ull); } gdbserver_state.signal = 0; gdb_continue(); } -static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_cont_with_sig(GArray *params, void *user_ctx) { unsigned long signal = 0; @@ -1604,8 +1586,8 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx) * Note: C sig;[addr] is currently unsupported and we simply * omit the addr parameter */ - if (gdb_ctx->num_params) { - signal = gdb_ctx->params[0].val_ul; + if (params->len) { + signal = get_param(params, 0)->val_ul; } gdbserver_state.signal = gdb_signal_to_target(signal); @@ -1615,27 +1597,27 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(); } -static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_set_thread(GArray *params, void *user_ctx) { CPUState *cpu; - if (gdb_ctx->num_params != 2) { + if (params->len != 2) { put_packet("E22"); return; } - if (gdb_ctx->params[1].thread_id.kind == GDB_READ_THREAD_ERR) { + if (get_param(params, 1)->thread_id.kind == GDB_READ_THREAD_ERR) { put_packet("E22"); return; } - if (gdb_ctx->params[1].thread_id.kind != GDB_ONE_THREAD) { + if (get_param(params, 1)->thread_id.kind != GDB_ONE_THREAD) { put_packet("OK"); return; } - cpu = gdb_get_cpu(gdb_ctx->params[1].thread_id.pid, - gdb_ctx->params[1].thread_id.tid); + cpu = gdb_get_cpu(get_param(params, 1)->thread_id.pid, + get_param(params, 1)->thread_id.tid); if (!cpu) { put_packet("E22"); return; @@ -1645,7 +1627,7 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx) * Note: This command is deprecated and modern gdb's will be using the * vCont command instead. */ - switch (gdb_ctx->params[0].opcode) { + switch (get_param(params, 0)->opcode) { case 'c': gdbserver_state.c_cpu = cpu; put_packet("OK"); @@ -1660,18 +1642,18 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx) } } -static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_insert_bp(GArray *params, void *user_ctx) { int res; - if (gdb_ctx->num_params != 3) { + if (params->len != 3) { put_packet("E22"); return; } - res = gdb_breakpoint_insert(gdb_ctx->params[0].val_ul, - gdb_ctx->params[1].val_ull, - gdb_ctx->params[2].val_ull); + res = gdb_breakpoint_insert(get_param(params, 0)->val_ul, + get_param(params, 1)->val_ull, + get_param(params, 2)->val_ull); if (res >= 0) { put_packet("OK"); return; @@ -1683,18 +1665,18 @@ static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet("E22"); } -static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_remove_bp(GArray *params, void *user_ctx) { int res; - if (gdb_ctx->num_params != 3) { + if (params->len != 3) { put_packet("E22"); return; } - res = gdb_breakpoint_remove(gdb_ctx->params[0].val_ul, - gdb_ctx->params[1].val_ull, - gdb_ctx->params[2].val_ull); + res = gdb_breakpoint_remove(get_param(params, 0)->val_ul, + get_param(params, 1)->val_ull, + get_param(params, 2)->val_ull); if (res >= 0) { put_packet("OK"); return; @@ -1717,7 +1699,7 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx) * the remote gdb to fallback to older methods. */ -static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_set_reg(GArray *params, void *user_ctx) { int reg_size; @@ -1726,19 +1708,19 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx) return; } - if (gdb_ctx->num_params != 2) { + if (params->len != 2) { put_packet("E22"); return; } - reg_size = strlen(gdb_ctx->params[1].data) / 2; - hextomem(gdbserver_state.mem_buf, gdb_ctx->params[1].data, reg_size); + reg_size = strlen(get_param(params, 1)->data) / 2; + hextomem(gdbserver_state.mem_buf, get_param(params, 1)->data, reg_size); gdb_write_register(gdbserver_state.g_cpu, gdbserver_state.mem_buf->data, - gdb_ctx->params[0].val_ull); + get_param(params, 0)->val_ull); put_packet("OK"); } -static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_get_reg(GArray *params, void *user_ctx) { int reg_size; @@ -1747,14 +1729,14 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) return; } - if (!gdb_ctx->num_params) { + if (!params->len) { put_packet("E14"); return; } reg_size = gdb_read_register(gdbserver_state.g_cpu, gdbserver_state.mem_buf, - gdb_ctx->params[0].val_ull); + get_param(params, 0)->val_ull); if (!reg_size) { put_packet("E14"); return; @@ -1766,22 +1748,24 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } -static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_write_mem(GArray *params, void *user_ctx) { - if (gdb_ctx->num_params != 3) { + if (params->len != 3) { put_packet("E22"); return; } /* hextomem() reads 2*len bytes */ - if (gdb_ctx->params[1].val_ull > strlen(gdb_ctx->params[2].data) / 2) { + if (get_param(params, 1)->val_ull > + strlen(get_param(params, 2)->data) / 2) { put_packet("E22"); return; } - hextomem(gdbserver_state.mem_buf, gdb_ctx->params[2].data, - gdb_ctx->params[1].val_ull); - if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull, + hextomem(gdbserver_state.mem_buf, get_param(params, 2)->data, + get_param(params, 1)->val_ull); + if (target_memory_rw_debug(gdbserver_state.g_cpu, + get_param(params, 0)->val_ull, gdbserver_state.mem_buf->data, gdbserver_state.mem_buf->len, true)) { put_packet("E14"); @@ -1791,22 +1775,24 @@ static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet("OK"); } -static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_read_mem(GArray *params, void *user_ctx) { - if (gdb_ctx->num_params != 2) { + if (params->len != 2) { put_packet("E22"); return; } /* memtohex() doubles the required space */ - if (gdb_ctx->params[1].val_ull > MAX_PACKET_LENGTH / 2) { + if (get_param(params, 1)->val_ull > MAX_PACKET_LENGTH / 2) { put_packet("E22"); return; } - g_byte_array_set_size(gdbserver_state.mem_buf, gdb_ctx->params[1].val_ull); + g_byte_array_set_size(gdbserver_state.mem_buf, + get_param(params, 1)->val_ull); - if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull, + if (target_memory_rw_debug(gdbserver_state.g_cpu, + get_param(params, 0)->val_ull, gdbserver_state.mem_buf->data, gdbserver_state.mem_buf->len, false)) { put_packet("E14"); @@ -1818,19 +1804,19 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } -static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_write_all_regs(GArray *params, void *user_ctx) { target_ulong addr, len; uint8_t *registers; int reg_size; - if (!gdb_ctx->num_params) { + if (!params->len) { return; } cpu_synchronize_state(gdbserver_state.g_cpu); - len = strlen(gdb_ctx->params[0].data) / 2; - hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len); + len = strlen(get_param(params, 0)->data) / 2; + hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len); registers = gdbserver_state.mem_buf->data; for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0; addr++) { @@ -1841,7 +1827,7 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet("OK"); } -static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_read_all_regs(GArray *params, void *user_ctx) { target_ulong addr, len; @@ -1859,14 +1845,14 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } -static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_file_io(GArray *params, void *user_ctx) { - if (gdb_ctx->num_params >= 1 && gdbserver_state.current_syscall_cb) { + if (params->len >= 1 && gdbserver_state.current_syscall_cb) { target_ulong ret, err; - ret = (target_ulong)gdb_ctx->params[0].val_ull; - if (gdb_ctx->num_params >= 2) { - err = (target_ulong)gdb_ctx->params[1].val_ull; + ret = (target_ulong)get_param(params, 0)->val_ull; + if (params->len >= 2) { + err = (target_ulong)get_param(params, 1)->val_ull; } else { err = 0; } @@ -1874,7 +1860,7 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) gdbserver_state.current_syscall_cb = NULL; } - if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') { + if (params->len >= 3 && get_param(params, 2)->opcode == (uint8_t)'C') { put_packet("T02"); return; } @@ -1882,23 +1868,23 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx) gdb_continue(); } -static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_step(GArray *params, void *user_ctx) { - if (gdb_ctx->num_params) { - gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull); + if (params->len) { + gdb_set_cpu_pc((target_ulong)get_param(params, 0)->val_ull); } cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags()); gdb_continue(); } -static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_backward(GArray *params, void *user_ctx) { if (replay_mode != REPLAY_MODE_PLAY) { put_packet("E22"); } - if (gdb_ctx->num_params == 1) { - switch (gdb_ctx->params[0].opcode) { + if (params->len == 1) { + switch (get_param(params, 0)->opcode) { case 's': if (replay_reverse_step()) { gdb_continue(); @@ -1920,20 +1906,20 @@ static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(""); } -static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_v_cont_query(GArray *params, void *user_ctx) { put_packet("vCont;c;C;s;S"); } -static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_v_cont(GArray *params, void *user_ctx) { int res; - if (!gdb_ctx->num_params) { + if (!params->len) { return; } - res = gdb_handle_vcont(gdb_ctx->params[0].data); + res = gdb_handle_vcont(get_param(params, 0)->data); if ((res == -EINVAL) || (res == -ERANGE)) { put_packet("E22"); } else if (res) { @@ -1941,17 +1927,17 @@ static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx) } } -static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_v_attach(GArray *params, void *user_ctx) { GDBProcess *process; CPUState *cpu; g_string_assign(gdbserver_state.str_buf, "E22"); - if (!gdb_ctx->num_params) { + if (!params->len) { goto cleanup; } - process = gdb_get_process(gdb_ctx->params[0].val_ul); + process = gdb_get_process(get_param(params, 0)->val_ul); if (!process) { goto cleanup; } @@ -1972,7 +1958,7 @@ cleanup: put_strbuf(); } -static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_v_kill(GArray *params, void *user_ctx) { /* Kill the target */ put_packet("OK"); @@ -2007,43 +1993,43 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = { }, }; -static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_v_commands(GArray *params, void *user_ctx) { - if (!gdb_ctx->num_params) { + if (!params->len) { return; } - if (process_string_cmd(NULL, gdb_ctx->params[0].data, + if (process_string_cmd(NULL, get_param(params, 0)->data, gdb_v_commands_table, ARRAY_SIZE(gdb_v_commands_table))) { put_packet(""); } } -static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_qemu_sstepbits(GArray *params, void *user_ctx) { g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER); put_strbuf(); } -static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_set_qemu_sstep(GArray *params, void *user_ctx) { - if (!gdb_ctx->num_params) { + if (!params->len) { return; } - sstep_flags = gdb_ctx->params[0].val_ul; + sstep_flags = get_param(params, 0)->val_ul; put_packet("OK"); } -static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_qemu_sstep(GArray *params, void *user_ctx) { g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags); put_strbuf(); } -static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_curr_tid(GArray *params, void *user_ctx) { CPUState *cpu; GDBProcess *process; @@ -2060,7 +2046,7 @@ static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } -static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_threads(GArray *params, void *user_ctx) { if (!gdbserver_state.query_cpu) { put_packet("l"); @@ -2073,25 +2059,25 @@ static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx) gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu); } -static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_first_threads(GArray *params, void *user_ctx) { gdbserver_state.query_cpu = gdb_first_attached_cpu(); - handle_query_threads(gdb_ctx, user_ctx); + handle_query_threads(params, user_ctx); } -static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_thread_extra(GArray *params, void *user_ctx) { g_autoptr(GString) rs = g_string_new(NULL); CPUState *cpu; - if (!gdb_ctx->num_params || - gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) { + if (!params->len || + get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) { put_packet("E22"); return; } - cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid, - gdb_ctx->params[0].thread_id.tid); + cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid, + get_param(params, 0)->thread_id.tid); if (!cpu) { return; } @@ -2116,7 +2102,7 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx) } #ifdef CONFIG_USER_ONLY -static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_offsets(GArray *params, void *user_ctx) { TaskState *ts; @@ -2131,17 +2117,17 @@ static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } #else -static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_rcmd(GArray *params, void *user_ctx) { const guint8 zero = 0; int len; - if (!gdb_ctx->num_params) { + if (!params->len) { put_packet("E22"); return; } - len = strlen(gdb_ctx->params[0].data); + len = strlen(get_param(params, 0)->data); if (len % 2) { put_packet("E01"); return; @@ -2149,7 +2135,7 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx) g_assert(gdbserver_state.mem_buf->len == 0); len = len / 2; - hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len); + hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len); g_byte_array_append(gdbserver_state.mem_buf, &zero, 1); qemu_chr_be_write(gdbserver_state.mon_chr, gdbserver_state.mem_buf->data, gdbserver_state.mem_buf->len); @@ -2157,7 +2143,7 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx) } #endif -static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_supported(GArray *params, void *user_ctx) { CPUClass *cc; @@ -2178,8 +2164,8 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) } #endif - if (gdb_ctx->num_params && - strstr(gdb_ctx->params[0].data, "multiprocess+")) { + if (params->len && + strstr(get_param(params, 0)->data, "multiprocess+")) { gdbserver_state.multiprocess = true; } @@ -2187,7 +2173,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx) put_strbuf(); } -static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_xfer_features(GArray *params, void *user_ctx) { GDBProcess *process; CPUClass *cc; @@ -2195,7 +2181,7 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx) const char *xml; const char *p; - if (gdb_ctx->num_params < 3) { + if (params->len < 3) { put_packet("E22"); return; } @@ -2208,15 +2194,15 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx) } gdb_has_xml = true; - p = gdb_ctx->params[0].data; + p = get_param(params, 0)->data; xml = get_feature_xml(p, &p, process); if (!xml) { put_packet("E00"); return; } - addr = gdb_ctx->params[1].val_ul; - len = gdb_ctx->params[2].val_ul; + addr = get_param(params, 1)->val_ul; + len = get_param(params, 2)->val_ul; total_len = strlen(xml); if (addr > total_len) { put_packet("E00"); @@ -2240,18 +2226,18 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx) } #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX_USER) -static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_xfer_auxv(GArray *params, void *user_ctx) { TaskState *ts; unsigned long offset, len, saved_auxv, auxv_len; - if (gdb_ctx->num_params < 2) { + if (params->len < 2) { put_packet("E22"); return; } - offset = gdb_ctx->params[0].val_ul; - len = gdb_ctx->params[1].val_ul; + offset = get_param(params, 0)->val_ul; + len = get_param(params, 1)->val_ul; ts = gdbserver_state.c_cpu->opaque; saved_auxv = ts->info->saved_auxv; auxv_len = ts->info->auxv_len; @@ -2286,12 +2272,12 @@ static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx) } #endif -static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_attached(GArray *params, void *user_ctx) { put_packet(GDB_ATTACHED); } -static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_query_qemu_supported(GArray *params, void *user_ctx) { g_string_printf(gdbserver_state.str_buf, "sstepbits;sstep"); #ifndef CONFIG_USER_ONLY @@ -2301,21 +2287,21 @@ static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx) } #ifndef CONFIG_USER_ONLY -static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, +static void handle_query_qemu_phy_mem_mode(GArray *params, void *user_ctx) { g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode); put_strbuf(); } -static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_set_qemu_phy_mem_mode(GArray *params, void *user_ctx) { - if (!gdb_ctx->num_params) { + if (!params->len) { put_packet("E22"); return; } - if (!gdb_ctx->params[0].val_ul) { + if (!get_param(params, 0)->val_ul) { phy_memory_mode = 0; } else { phy_memory_mode = 1; @@ -2438,45 +2424,45 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = { #endif }; -static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_gen_query(GArray *params, void *user_ctx) { - if (!gdb_ctx->num_params) { + if (!params->len) { return; } - if (!process_string_cmd(NULL, gdb_ctx->params[0].data, + if (!process_string_cmd(NULL, get_param(params, 0)->data, gdb_gen_query_set_common_table, ARRAY_SIZE(gdb_gen_query_set_common_table))) { return; } - if (process_string_cmd(NULL, gdb_ctx->params[0].data, + if (process_string_cmd(NULL, get_param(params, 0)->data, gdb_gen_query_table, ARRAY_SIZE(gdb_gen_query_table))) { put_packet(""); } } -static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_gen_set(GArray *params, void *user_ctx) { - if (!gdb_ctx->num_params) { + if (!params->len) { return; } - if (!process_string_cmd(NULL, gdb_ctx->params[0].data, + if (!process_string_cmd(NULL, get_param(params, 0)->data, gdb_gen_query_set_common_table, ARRAY_SIZE(gdb_gen_query_set_common_table))) { return; } - if (process_string_cmd(NULL, gdb_ctx->params[0].data, + if (process_string_cmd(NULL, get_param(params, 0)->data, gdb_gen_set_table, ARRAY_SIZE(gdb_gen_set_table))) { put_packet(""); } } -static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx) +static void handle_target_halt(GArray *params, void *user_ctx) { g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP); gdb_append_thread_id(gdbserver_state.c_cpu, gdbserver_state.str_buf); From ae49ce00198a46f635d5a37ab79209c145f7960f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 20 May 2021 18:43:01 +0100 Subject: [PATCH 5/7] hmp-commands: expand type of icount to "l" in replay commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not a 32 bit number, it can (and most likely will) be quite a big one. Signed-off-by: Alex Bennée Reviewed-by: Pavel Dovgalyuk Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210520174303.12310-7-alex.bennee@linaro.org> --- hmp-commands.hx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 435c591a1c..5ee9cfd520 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1667,7 +1667,7 @@ ERST { .name = "replay_break", - .args_type = "icount:i", + .args_type = "icount:l", .params = "icount", .help = "set breakpoint at the specified instruction count", .cmd = hmp_replay_break, @@ -1699,7 +1699,7 @@ ERST { .name = "replay_seek", - .args_type = "icount:i", + .args_type = "icount:l", .params = "icount", .help = "replay execution to the specified instruction count", .cmd = hmp_replay_seek, From ed12f5b4ef626bfd1deb6f178282a18ab948750e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 20 May 2021 18:43:02 +0100 Subject: [PATCH 6/7] gdbstub: tidy away reverse debugging check into function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In theory we don't need an actual record/replay to enact reverse debugging on a purely deterministic system (i.e one with no external inputs running under icount). Tidy away the logic into a little function. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210520174303.12310-8-alex.bennee@linaro.org> --- gdbstub.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 84ce770a04..52bde5bdc9 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -465,6 +465,15 @@ int use_gdb_syscalls(void) return gdb_syscall_mode == GDB_SYS_ENABLED; } +static bool stub_can_reverse(void) +{ +#ifdef CONFIG_USER_ONLY + return false; +#else + return replay_mode == REPLAY_MODE_PLAY; +#endif +} + /* Resume execution. */ static inline void gdb_continue(void) { @@ -1880,7 +1889,7 @@ static void handle_step(GArray *params, void *user_ctx) static void handle_backward(GArray *params, void *user_ctx) { - if (replay_mode != REPLAY_MODE_PLAY) { + if (!stub_can_reverse()) { put_packet("E22"); } if (params->len == 1) { @@ -2153,7 +2162,7 @@ static void handle_query_supported(GArray *params, void *user_ctx) g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+"); } - if (replay_mode == REPLAY_MODE_PLAY) { + if (stub_can_reverse()) { g_string_append(gdbserver_state.str_buf, ";ReverseStep+;ReverseContinue+"); } From a6851b49e3b0509168d74050d0e59fe0121b2898 Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Thu, 20 May 2021 18:43:03 +0100 Subject: [PATCH 7/7] plugins/syscall: Added a table-like summary output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a table-like output which contains the total number of calls for each used syscall along with the number of errors that occurred. Per-call tracing is still available through supplying the argument ``print`` to the plugin. Signed-off-by: Mahmoud Mandour Signed-off-by: Alex Bennée Message-Id: <20210519032409.3041-1-ma.mandourr@gmail.com> Message-Id: <20210520174303.12310-9-alex.bennee@linaro.org> --- tests/plugin/syscall.c | 100 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 6 deletions(-) diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c index 53ee2ab6c4..6dd71092e1 100644 --- a/tests/plugin/syscall.c +++ b/tests/plugin/syscall.c @@ -16,32 +16,120 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; +typedef struct { + int64_t num; + int64_t calls; + int64_t errors; +} SyscallStats; + +static GMutex lock; +static GHashTable *statistics; + +static SyscallStats *get_or_create_entry(int64_t num) +{ + SyscallStats *entry = + (SyscallStats *) g_hash_table_lookup(statistics, GINT_TO_POINTER(num)); + + if (!entry) { + entry = g_new0(SyscallStats, 1); + entry->num = num; + g_hash_table_insert(statistics, GINT_TO_POINTER(num), (gpointer) entry); + } + + return entry; +} + static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index, int64_t num, uint64_t a1, uint64_t a2, uint64_t a3, uint64_t a4, uint64_t a5, uint64_t a6, uint64_t a7, uint64_t a8) { - g_autofree gchar *out = g_strdup_printf("syscall #%" PRIi64 "\n", num); - qemu_plugin_outs(out); + if (statistics) { + SyscallStats *entry; + g_mutex_lock(&lock); + entry = get_or_create_entry(num); + entry->calls++; + g_mutex_unlock(&lock); + } else { + g_autofree gchar *out = g_strdup_printf("syscall #%" PRIi64 "\n", num); + qemu_plugin_outs(out); + } } static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx, int64_t num, int64_t ret) +{ + if (statistics) { + SyscallStats *entry; + + g_mutex_lock(&lock); + /* Should always return an existent entry. */ + entry = get_or_create_entry(num); + if (ret < 0) { + entry->errors++; + } + g_mutex_unlock(&lock); + } else { + g_autofree gchar *out; + out = g_strdup_printf("syscall #%" PRIi64 " returned -> %" PRIi64 "\n", + num, ret); + qemu_plugin_outs(out); + } +} + +static void print_entry(gpointer val, gpointer user_data) { g_autofree gchar *out; - out = g_strdup_printf("syscall #%" PRIi64 " returned -> %" PRIi64 "\n", - num, ret); + SyscallStats *entry = (SyscallStats *) val; + int64_t syscall_num = entry->num; + out = g_strdup_printf( + "%-13" PRIi64 "%-6" PRIi64 " %" PRIi64 "\n", + syscall_num, entry->calls, entry->errors); qemu_plugin_outs(out); } -/* ************************************************************************* */ +static gint comp_func(gconstpointer ea, gconstpointer eb) +{ + SyscallStats *ent_a = (SyscallStats *) ea; + SyscallStats *ent_b = (SyscallStats *) eb; -static void plugin_exit(qemu_plugin_id_t id, void *p) {} + return ent_a->calls > ent_b->calls ? -1 : 1; +} + +/* ************************************************************************* */ +static void plugin_exit(qemu_plugin_id_t id, void *p) +{ + if (!statistics) { + return; + } + + g_mutex_lock(&lock); + GList *entries = g_hash_table_get_values(statistics); + entries = g_list_sort(entries, comp_func); + qemu_plugin_outs("syscall no. calls errors\n"); + + g_list_foreach(entries, print_entry, NULL); + + g_list_free(entries); + g_hash_table_destroy(statistics); + g_mutex_unlock(&lock); +} QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, int argc, char **argv) { + if (argc == 0) { + statistics = g_hash_table_new_full(NULL, g_direct_equal, NULL, g_free); + } else { + for (int i = 0; i < argc; i++) { + if (g_strcmp0(argv[i], "print") != 0) { + fprintf(stderr, "unsupported argument: %s\n", argv[i]); + return -1; + } + } + } + qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall); qemu_plugin_register_vcpu_syscall_ret_cb(id, vcpu_syscall_ret); qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);