From patchwork Tue Aug 29 16:15:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 718221 Delivered-To: patch@linaro.org Received: by 2002:adf:d20a:0:b0:31d:da82:a3b4 with SMTP id j10csp67885wrh; Tue, 29 Aug 2023 09:16:58 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG7N2gH9WFVypZdParIMSYLjNpXfKwYBoWgCj/nu6ltA+Ausrl+5KUtx3XH5PRBaf09PKSZ X-Received: by 2002:a05:6214:12da:b0:64a:3320:8a8d with SMTP id s26-20020a05621412da00b0064a33208a8dmr3676750qvv.30.1693325818418; Tue, 29 Aug 2023 09:16:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1693325818; cv=none; d=google.com; s=arc-20160816; b=hpkt0GkJdVFS+HT1f5yw9C2Ji01lKPS6AFADppvhAgRY/qXWZ299stbsPr6oxhiJGo CbkeGcFDgA885c4o1K7VLhnoibV5knkHWuO+yCEh/KO0vTVO6jJA88LQdRx5Dm3s/uEx zeQzFBQHuSgfLeIh8EMpBR6GuzwgRdFgrFLGtqtsS5llrz44e9gJYj2HT1wK/QNKJPP3 BX6TDifl5+nLfVV/Hi+jRiAPIHws8SIkVrKnLKMneczam8UMpdllZN2o2Z2/sYDknvly tYT9cVEyk7B/lTOfDeKCkCNexGWLv3xYSFf6tV99JT1JgbASM/mbU1BNIMkjE7J3sQYl tYJQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=M3HpoGymUAOECIzpXws+SoFW7NCUK3OvK2CLae4WjpQ=; fh=jHSoIYwJilbDFQToVbFas5A/XFwuejwyQpYY568nC5c=; b=pQCGZeRRJRLRMkTVkq8hv8QQ9JYdBgK4nBCxVAztnmFOpQ0ka1WmUe/L2WKE5/6bm0 Ida8MvMnxjWV8YaU77v172vpuFSA3F+uCev77XmQVNrjNz3Dns/IgQLEN6/WHsEVUKg+ ZmxOk25k8WYpq2MxTO+Gz6y3fegIwQm0LxY8Zcv7tC4fyoVuARyqcvc9i9AOI/f7POCw k7yEN7NZN6dfDeApIzXH9FmeydHZ2kKKCdyfd/SVzZOtYGicDi3ac0kJFr5HcY/qQHPP p5IpRC5JgbamHmS57XEDU3VYS9VuYuLZ5VcYyl/PfIg+Aj1hiTaAkoPA54IegKFM7TEv dVDg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=HC+Uzw4p; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id e8-20020a056214162800b0064f9333d5d5si4009843qvw.427.2023.08.29.09.16.58 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 29 Aug 2023 09:16:58 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=HC+Uzw4p; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qb1Nr-0005AN-Ct; Tue, 29 Aug 2023 12:16:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qb1Nk-00050D-Nf for qemu-devel@nongnu.org; Tue, 29 Aug 2023 12:15:56 -0400 Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qb1NT-0004Pj-QS for qemu-devel@nongnu.org; Tue, 29 Aug 2023 12:15:56 -0400 Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-400e8ca7e38so40109435e9.3 for ; Tue, 29 Aug 2023 09:15:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1693325732; x=1693930532; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=M3HpoGymUAOECIzpXws+SoFW7NCUK3OvK2CLae4WjpQ=; b=HC+Uzw4pJ58SrxhsrVx0KmFa/C21po8OGLJYGH4et8xnoSBCqw3Y88JvWxapRKKttF uRyUyWQmjlOPlh0oQyUIT9LJXcp14boa3FmYJYZxoF8mueuR2yIkcGIarmZ7xYCWKnpH bB3CUH8sARI0FmzaEHa0dS5JMOkeQIcKrEwSjk3I5E4DGwMN7RFADALfiCu1UxWzb1MD 0gGFvlpcReZD0KWNQfNrpHxnjgF1WNKPSP6Ia1tOpiI5bYaqBCWFjlMik/n3wQojSWh/ 7nZn2VljQGidgZUtLnDOYc9fRI0JCEd0xO2ZS0EyTbPXLBCc0BLJ2AR9dIWw73qnk9gw Am9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693325732; x=1693930532; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=M3HpoGymUAOECIzpXws+SoFW7NCUK3OvK2CLae4WjpQ=; b=ANsM3vnFZE4VmiLI+1opZy89LYx58RtxUmiV46IvRINoaWbMAixxAqvcwBNKtYiGf+ cTFhaF4kuTnraQqdjka3RPa+GCDDp6TpFOeqJH+L/smikDuvkssxxAufUYLkSk5lUlRV v2jndMAr2V7xx9+DRWs/SEz7xvJ7DpDMB6ycT8dUzGTjP1NZlg8k6eXlXxfA0vu34oUR NLB/Sfcr0AN+fnaTdl2uriGWcm21kALD3YSv/+lNQ1iMuYYcEmrD7wMhiwv68cgBSfoI owQfmgMQXJa1OLP9DMOCQ47/8eA2aMjMRY/ClUVwtXAUwIFyne1kNuUy+14BEl13PmCA NlwA== X-Gm-Message-State: AOJu0YyonNECVsZ/pGnk5EjS3qyyil+dsN2Wp9jvxfiq/Uag4TegGOF9 O4EXd4sm3mc6hfEglumURVZ+Kw== X-Received: by 2002:a7b:c3cf:0:b0:401:6800:7032 with SMTP id t15-20020a7bc3cf000000b0040168007032mr11574070wmj.18.1693325732269; Tue, 29 Aug 2023 09:15:32 -0700 (PDT) Received: from zen.linaroharston ([85.9.250.243]) by smtp.gmail.com with ESMTPSA id o23-20020a5d58d7000000b0031980783d78sm14210412wrf.54.2023.08.29.09.15.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Aug 2023 09:15:30 -0700 (PDT) Received: from zen.lan (localhost [127.0.0.1]) by zen.linaroharston (Postfix) with ESMTP id 30B2E1FFC3; Tue, 29 Aug 2023 17:15:29 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: Wainer dos Santos Moschetta , =?utf-8?q?Daniel_P=2E?= =?utf-8?q?_Berrang=C3=A9?= , =?utf-8?q?Philippe_Mathie?= =?utf-8?q?u-Daud=C3=A9?= , David Gibson , =?utf-8?q?C=C3=A9dric_Le_Goater?= , Daniel Henrique Barboza , qemu-arm@nongnu.org, Markus Armbruster , qemu-ppc@nongnu.org, David Hildenbrand , qemu-s390x@nongnu.org, Greg Kurz , Nicholas Piggin , Akihiko Odaki , Juan Quintela , Yonggang Luo , Richard Henderson , Ilya Leoshkevich , Peter Maydell , Beraldo Leal , =?utf-8?q?Alex_Benn=C3=A9e?= , Thomas Huth , Matheus Branco Borella Subject: [PATCH v3 08/12] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT Date: Tue, 29 Aug 2023 17:15:24 +0100 Message-Id: <20230829161528.2707696-9-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230829161528.2707696-1-alex.bennee@linaro.org> References: <20230829161528.2707696-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::32b; envelope-from=alex.bennee@linaro.org; helo=mail-wm1-x32b.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org From: Matheus Branco Borella This fix is implemented by having the vCont handler set the value of `gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU picked is arbitrarily from the ones to be resumed, but it should be okay, as all GDB cares about is that it is a resumed thread. Signed-off-by: Matheus Branco Borella Message-Id: <20230804182633.47300-2-dark.ryu.550@gmail.com> [AJB: style and whitespace fixes] Signed-off-by: Alex Bennée Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725 --- v2 - fix up some whitespace --- gdbstub/gdbstub.c | 29 ++++++ tests/tcg/multiarch/system/interrupt.c | 28 ++++++ tests/tcg/multiarch/gdbstub/interrupt.py | 97 +++++++++++++++++++ .../multiarch/system/Makefile.softmmu-target | 12 ++- 4 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/multiarch/system/interrupt.c create mode 100644 tests/tcg/multiarch/gdbstub/interrupt.py diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 5f28d5cf57..e7d48fa0d4 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -597,6 +597,15 @@ static int gdb_handle_vcont(const char *p) * or incorrect parameters passed. */ res = 0; + + /* + * target_count and last_target keep track of how many CPUs we are going to + * step or resume, and a pointer to the state structure of one of them, + * respectivelly + */ + int target_count = 0; + CPUState *last_target = NULL; + while (*p) { if (*p++ != ';') { return -ENOTSUP; @@ -637,6 +646,9 @@ static int gdb_handle_vcont(const char *p) while (cpu) { if (newstates[cpu->cpu_index] == 1) { newstates[cpu->cpu_index] = cur_action; + + target_count++; + last_target = cpu; } cpu = gdb_next_attached_cpu(cpu); @@ -654,6 +666,9 @@ static int gdb_handle_vcont(const char *p) while (cpu) { if (newstates[cpu->cpu_index] == 1) { newstates[cpu->cpu_index] = cur_action; + + target_count++; + last_target = cpu; } cpu = gdb_next_cpu_in_process(cpu); @@ -671,11 +686,25 @@ static int gdb_handle_vcont(const char *p) /* only use if no previous match occourred */ if (newstates[cpu->cpu_index] == 1) { newstates[cpu->cpu_index] = cur_action; + + target_count++; + last_target = cpu; } break; } } + /* + * if we're about to resume a specific set of CPUs/threads, make it so that + * in case execution gets interrupted, we can send GDB a stop reply with a + * correct value. it doesn't really matter which CPU we tell GDB the signal + * happened in (VM pauses stop all of them anyway), so long as it is one of + * the ones we resumed/single stepped here. + */ + if (target_count > 0) { + gdbserver_state.c_cpu = last_target; + } + gdbserver_state.signal = signal; gdb_continue_partial(newstates); return res; diff --git a/tests/tcg/multiarch/system/interrupt.c b/tests/tcg/multiarch/system/interrupt.c new file mode 100644 index 0000000000..98d4f2eff9 --- /dev/null +++ b/tests/tcg/multiarch/system/interrupt.c @@ -0,0 +1,28 @@ +/* + * External interruption test. This test is structured in such a way that it + * passes the cases that require it to exit, but we can make it enter an + * infinite loop from GDB. + * + * We don't have the benefit of libc, just builtin C primitives and + * whatever is in minilib. + */ + +#include + +void loop(void) +{ + do { + /* + * Loop forever. Just make sure the condition is always a constant + * expression, so that this loop is not UB, as per the C + * standard. + */ + } while (1); +} + +int main(void) +{ + return 0; +} + + diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py new file mode 100644 index 0000000000..e222ac94c5 --- /dev/null +++ b/tests/tcg/multiarch/gdbstub/interrupt.py @@ -0,0 +1,97 @@ +from __future__ import print_function +# +# Test some of the softmmu debug features with the multiarch memory +# test. It is a port of the original vmlinux focused test case but +# using the "memory" test instead. +# +# This is launched via tests/guest-debug/run-test.py +# + +import gdb +import sys + +failcount = 0 + + +def report(cond, msg): + "Report success/fail of test" + if cond: + print("PASS: %s" % (msg)) + else: + print("FAIL: %s" % (msg)) + global failcount + failcount += 1 + + +def check_interrupt(thread): + """ + Check that, if thread is resumed, we go back to the same thread when the + program gets interrupted. + """ + + # Switch to the thread we're going to be running the test in. + print("thread ", thread.num) + gdb.execute("thr %d" % thread.num) + + # Enter the loop() function on this thread. + # + # While there are cleaner ways to do this, we want to minimize the number of + # side effects on the gdbstub's internal state, since those may mask bugs. + # Ideally, there should be no difference between what we're doing here and + # the program reaching the loop() function on its own. + # + # For this to be safe, we only need the prologue of loop() to not have + # instructions that may have problems with what we're doing here. We don't + # have to worry about anything else, as this function never returns. + gdb.execute("set $pc = loop") + + # Continue and then interrupt the task. + gdb.post_event(lambda: gdb.execute("interrupt")) + gdb.execute("c") + + # Check whether the thread we're in after the interruption is the same we + # ran continue from. + return (thread.num == gdb.selected_thread().num) + + +def run_test(): + """ + Test if interrupting the code always lands us on the same thread when + running with scheduler-lock enabled. + """ + + gdb.execute("set scheduler-locking on") + for thread in gdb.selected_inferior().threads(): + report(check_interrupt(thread), + "thread %d resumes correctly on interrupt" % thread.num) + + +# +# This runs as the script it sourced (via -x, via run-test.py) +# +try: + inferior = gdb.selected_inferior() + arch = inferior.architecture() + print("ATTACHED: %s" % arch.name()) +except (gdb.error, AttributeError): + print("SKIPPING (not connected)", file=sys.stderr) + exit(0) + +if gdb.parse_and_eval('$pc') == 0: + print("SKIP: PC not set") + exit(0) +if len(gdb.selected_inferior().threads()) == 1: + print("SKIP: set to run on a single thread") + exit(0) + +try: + # Run the actual tests + run_test() +except (gdb.error): + print("GDB Exception: %s" % (sys.exc_info()[0])) + failcount += 1 + pass + +# Finally kill the inferior and exit gdb with a count of failures +gdb.execute("kill") +exit(failcount) diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target index a051d689d7..90810a32b2 100644 --- a/tests/tcg/multiarch/system/Makefile.softmmu-target +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target @@ -27,7 +27,15 @@ run-gdbstub-memory: memory "-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \ --bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \ softmmu gdbstub support) - +run-gdbstub-interrupt: interrupt + $(call run-test, $@, $(GDB_SCRIPT) \ + --gdb $(HAVE_GDB_BIN) \ + --qemu $(QEMU) \ + --output $<.gdb.out \ + --qargs \ + "-smp 2 -monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \ + --bin $< --test $(MULTIARCH_SRC)/gdbstub/interrupt.py, \ + softmmu gdbstub support) run-gdbstub-untimely-packet: hello $(call run-test, $@, $(GDB_SCRIPT) \ --gdb $(HAVE_GDB_BIN) \ @@ -50,4 +58,4 @@ run-gdbstub-%: $(call skip-test, "gdbstub test $*", "need working gdb") endif -MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-untimely-packet +MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet