From patchwork Thu Aug 24 16:39:06 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: 716552 Delivered-To: patch@linaro.org Received: by 2002:adf:f747:0:b0:317:ecd7:513f with SMTP id z7csp1444022wrp; Thu, 24 Aug 2023 09:41:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHMmeH5UmwdC0bJux6ZL4y9p2sRWbYReDgjeMFOTmzEXrsr42b7GbMgk8nqlEWYJX7H/d24 X-Received: by 2002:ac8:7d4f:0:b0:410:9a39:d3c1 with SMTP id h15-20020ac87d4f000000b004109a39d3c1mr13533274qtb.63.1692895281662; Thu, 24 Aug 2023 09:41:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692895281; cv=none; d=google.com; s=arc-20160816; b=tmR6K6jxGhi8mjjS644XB6KTwvl607QvLh4zBC2Jzy03ap/IuM03OfHSJkbRGJV1Tr 2jfjiU7c60fMprLJQUK74X/x+lUO6EqC90x367fLcWjxEK7fA86aKR+kKhJes99zQ4WY RTdKGJ2trBwfauRAbAbloNZ6ujy5Q8gxSeylK2D05bogZbWAiaUkIMuq4LUY2TZlC/V/ 8vM4dB6mHntqNG7sU58FZNLxZp2sXHHKSD3nTnv8SAtyjETl9vnskE1Tf+LHfP042NHl UpgRCFRSBZpgyqvnFc0RLCk1onh1m6zu5l9v7VGF5p0guP3LppnlJJ6il6IdPE0b0kFO XJNQ== 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=7oKnyFFGjQoIR1wptW5c56TGLTNRojjUyNfUZ3E+vOg=; b=0XYLZp4NrQ9EwFlGjE6c9eFa4d1EI4zneielg8BaMQvuXXAEFA9NUYawxzijK93Gi4 9rz6A3OsiIL2IlPNiiFOHDfRL07E8zSZyckcUJRjLLpM3uaNSSSqFZGWOvs/Y8r4FPdz Np4AkudoUoA1W//M1hLWDD9Vo0/Ppk77wAKH1oQy/lcV7Af5HphL9ZbFHPD7zLEQRQxw GLstPsEHW+CrhSjm6KvqqR2WYAic/qsaVhDBzuucdpKD0C0401fMsfFfk/n1jSG38lMo wL0XTVoFtVx0dDOfLDxVyvS4TrnkEoqPLdPE6YHHnhMgETWAFKCnkqio+bS0OkeWFsxy 37hw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=g1OWyylX; 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 b3-20020ac87fc3000000b004055d07b98bsi3531918qtk.321.2023.08.24.09.41.21 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 24 Aug 2023 09:41:21 -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=g1OWyylX; 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 1qZDMp-0006QP-TF; Thu, 24 Aug 2023 12:39:31 -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 1qZDMn-0006O8-SX for qemu-devel@nongnu.org; Thu, 24 Aug 2023 12:39:29 -0400 Received: from mail-wm1-x32e.google.com ([2a00:1450:4864:20::32e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qZDMb-00087U-GD for qemu-devel@nongnu.org; Thu, 24 Aug 2023 12:39:29 -0400 Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-3fed6c2a5cfso59323045e9.3 for ; Thu, 24 Aug 2023 09:39:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1692895156; x=1693499956; 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=g1OWyylXHUniLRMmrogJ5c+EJkbvWWpazU6rgmjIIKnC0ENMBE5QYApg7ayL/QTvqP CRAHdUMU4AheXY+wcDGpPPOOfPJp9GSIQZPwKM+pCwNCMb9d4vZD0MOdPTQYhUKLTJyJ o04vKz/MJjvnmA+wpZpa2q/oB5a3oe4+a1MPR048yV+o08IIllVkcWXEkq2s3YkJ6YTr gkcadQJxkOZ39Xoco4pplQqemoKHZUEIdoDZZQN50kdqaBdWgMO+D4Ml9Q2xuIQHJpGi jp6Rh0y8SUOuFw9vytPt0MAAeBVJdv8BqdxsqIOe1n350FgWkrErRRKxM6k6GKL7JPvL JHPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692895156; x=1693499956; 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=Ei9OZaLSslZMFUTaAwSulAjwuGYRcCqdLdds2j7o1hwlKdaTBRvLcv+HKeHmv9Inj2 uRA0Vh3ZcIZ3JBANG3KbVd7a3NM4Iukg1dQtVF4pwj9HzVw54NyEf3iEs45C/sw6ApHM PaIbK1a+X4idx06w7/xc9E1fTOAO3dwle6Hwm66Rw1EWHMOCkTM3/AfQq29LfJyTHxau Zzrh4MbxfDtvRn7Gfl8Djc33OGdCL/4ScKoyeRTsRPlGskreCZ23WBWFAdU9COcPD6N7 nzbjjKpnq5CHCH9AX5v5Nc6ydzqClPddE8X86NwYVqn05MoNm10zwRI8g8+bOoSkIz2L wj7A== X-Gm-Message-State: AOJu0YyR+iIZcMG+z28K4wU60iDCzVQijqhVt+TxmCi86lQo0kPqEy7r JCeL6mJFgIq7GFRIGmNALFWFhA== X-Received: by 2002:a05:600c:2291:b0:3f9:b8df:26ae with SMTP id 17-20020a05600c229100b003f9b8df26aemr12135286wmf.34.1692895155856; Thu, 24 Aug 2023 09:39:15 -0700 (PDT) Received: from zen.linaroharston ([85.9.250.243]) by smtp.gmail.com with ESMTPSA id u2-20020a05600c00c200b003fe17901fcdsm3091345wmm.32.2023.08.24.09.39.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Aug 2023 09:39:14 -0700 (PDT) Received: from zen.lan (localhost [127.0.0.1]) by zen.linaroharston (Postfix) with ESMTP id 35BC01FFC3; Thu, 24 Aug 2023 17:39:11 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: Beraldo Leal , Nicholas Piggin , Richard Henderson , Markus Armbruster , Yonggang Luo , qemu-ppc@nongnu.org, David Gibson , Daniel Henrique Barboza , qemu-s390x@nongnu.org, Akihiko Odaki , =?utf-8?q?Alex_Benn=C3=A9e?= , qemu-arm@nongnu.org, Juan Quintela , Thomas Huth , Peter Maydell , Ilya Leoshkevich , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , David Hildenbrand , Greg Kurz , Wainer dos Santos Moschetta , =?utf-8?q?Philippe_Mathi?= =?utf-8?q?eu-Daud=C3=A9?= , =?utf-8?q?C=C3=A9dric_Le_Goa?= =?utf-8?q?ter?= , Matheus Branco Borella Subject: [PATCH v2 08/12] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT Date: Thu, 24 Aug 2023 17:39:06 +0100 Message-Id: <20230824163910.1737079-9-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230824163910.1737079-1-alex.bennee@linaro.org> References: <20230824163910.1737079-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::32e; envelope-from=alex.bennee@linaro.org; helo=mail-wm1-x32e.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