From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id C1HJNmGOhmabHBsAWB0awg (envelope-from ) for ; Thu, 04 Jul 2024 07:58:25 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=DxwKPFKe; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id C174E1E0C3; Thu, 4 Jul 2024 07:58:25 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A4C5F1E097 for ; Thu, 4 Jul 2024 07:58:23 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 24210384A4A1 for ; Thu, 4 Jul 2024 11:58:22 +0000 (GMT) Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by sourceware.org (Postfix) with ESMTPS id B15773858C41 for ; Thu, 4 Jul 2024 11:57:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B15773858C41 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B15773858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::1029 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720094282; cv=none; b=LRQUiQIlTIAqe6Q5qSImukQxEVcNnAvH2nAfnK+vd7aQxLvB6zy4LXw9828578ypyjQdE0sqDsOAjzujR1ohjnSyamuDncGGAdSeN01MTScEL1v+6+v3Z7+JvGLy4XDMnbjhGRO8f3hubJ3Lmmx3qNkdn2+zZCmM8A+oe2/1G1g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720094282; c=relaxed/simple; bh=uZ+2LrcDcUH/Rb4W/u+tLsqb6nZspHEPk5ZYzEWXAAs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=hGkbCJP/1CkMpnqr/qsDiQVDdE3BSMWU7zytX7ZLjkFA1lzi2O2bTXxLCdMpwnZRtLXAuRPEPGXUGOkjBuEEtBSGHJZO5mCi0Z+FHklgtQc8yyL7C7ibLg5nwW7nfLQZQ7Xl9dV5MXYCZAVT9cWY5CXjvnKWZtixsd6wWbLA3uY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x1029.google.com with SMTP id 98e67ed59e1d1-2c2dee9d9cfso390753a91.3 for ; Thu, 04 Jul 2024 04:57:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720094278; x=1720699078; darn=sourceware.org; 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=N2gzCjICDw0l+0I4TYkJJqwPjYq9Mt2Dvf4Xgovx13U=; b=DxwKPFKeRJ1On0d/GM7x++hcSvAif/tGT3n0pjBv0llMIgFpG2ji8rUhiU3v1XhcxU tCy/FnB8m5rqV4wDmiQbJHWrC5IR9nfT2BkbZciuM7gelnNBce+Zi5Qf45eReyLKFRBE fLW+TJO2qqZ1C5TP+4xRbdgvYSg2V30fx9Xheadwrjx+WdiWW/15gYvBgyGQqe9v9Mi5 pUflufpVKsk3+wP1MS6kW3fjexPoi7bNELWTMnUOCK5MStXv/YK5QOutxTyUajIjthfk O7rWKRWBFqXhSERWrDMuNKGGANk5uCa8eWVvf2bRmhzvcCBsB6UT+SyDVqh9UMPOBy4o kFqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720094278; x=1720699078; 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=N2gzCjICDw0l+0I4TYkJJqwPjYq9Mt2Dvf4Xgovx13U=; b=FxdtcsVtZrsGWzFhzEJ7tWbaJfhcZrikyfu/hgfCg7gM5fkuVlPoCJVpFtWL2Y5Gvu nJA1guS2uLuRjhTfG2cckmLTxPPEreRfAwFx89wBv9MP5frBtA77YuamTlfUo7TlWFMf 1AmqYMxJecKIiAOttQiVmHiemjJToPrj7XKoA2+n5/SvCYt8QkW6bOIrhHqmr2wGyJZc UqWHcc4H4wObXO5zdMXcDs/b7nFAUH6hPw5bX8ttFcHKX7OpsL3HLQYl6yGkA9YWpcps Z2/w4nTl+apzwNb+apeJJqMlxwhO1NJMV8/lwl/zP57mr9YGQMJXuxi0B7f/ONuPbAW0 LovQ== X-Gm-Message-State: AOJu0Yxz20loItZFqLkAA6z1kRvA7q3p2liqnbQ7M3vRQIisZsjkBVRn UbuI7WFkv67561KdSZHvBdAWMT2DJKbyfNdL0E0uWIFGTJZ+XlMawXIasQ== X-Google-Smtp-Source: AGHT+IHTD0gw/rsIv9kf7zCahS21M8er/henSgqzxFM0ezFBzdLP9MyiX6gKP4Hl6h6w/6AuyuFApg== X-Received: by 2002:a17:90b:fc2:b0:2c2:c91c:9242 with SMTP id 98e67ed59e1d1-2c99c8046fcmr1082489a91.31.1720094277746; Thu, 04 Jul 2024 04:57:57 -0700 (PDT) Received: from valdaarhun.localnet ([182.48.210.74]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2c99a92a430sm1339733a91.4.2024.07.04.04.57.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Jul 2024 04:57:57 -0700 (PDT) From: Sahil To: gdb-patches@sourceware.org, Guinevere Larsen Cc: Sahil Siddiq Subject: Re: [PATCH v2] gdb: fix "frame function" issue when call is last instruction Date: Thu, 04 Jul 2024 17:27:53 +0530 Message-ID: <2958551.e9J7NaK4W3@valdaarhun> In-Reply-To: <729a2a2f-d8d2-4f6a-a34c-781c0b8410d0@redhat.com> References: <20240701195321.16591-1-sahilcdq@proton.me> <729a2a2f-d8d2-4f6a-a34c-781c0b8410d0@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_SBL_CSS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Hi, Thank you for the review. On Wednesday, July 3, 2024 7:54:43=E2=80=AFPM GMT+5:30 Guinevere Larsen wro= te: > [...] > I would prefer if you wrote "the last instruction in a frame is a call" > or something similar. I ask this because when I read the commit message, > I thought "last instruction" was referring to last instruction executed, > and couldn't understand why I couldn't reproduce without __builtin_abort. >=20 Sorry about that. I should have been clearer. I'll modify the commit messag= e. > > Using "get_frame_address_in_block" instead of > > "get_frame_pc" resolves this issue. > >=20 > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30929 > > --- > > Changes since v1: > > * frame-selection-last-instr-call.exp: Modify test to fix regressions on > > arm/aarch64 > >=20 > > Patch v1: > > https://sourceware.org/pipermail/gdb-patches/2024-June/210251.html > And responding to your v1 comment about why you created a new test file, > you mention creating a new C file for this test. Have you tried > extending the original frame-selection.c file with a new function that > calls __builtin_abort instead? I tried extending frame_selection.c initially but I was uncertain about adding a function that calls __builtin_abort since "main" would never return. I thought the last statement in main - "return i + j" - also plays a role in the test. After taking a look at the test again, I don't think that should cause a hindrance. I'll experiment with this. > It seems to me that it should work, and > you should be able to extend the existing frame-selection.exp file, but > I might be missing something. >=20 > I would prefer if we just extended the test, instead of added a new one, > but if I indeed missed something and it isn't as easy to extend it as I > think, we can have the new test :) >=20 I don't think extending frame-selection.exp should be difficult. I was main= ly unsure about frame-selection.c. > [...] > > diff --git a/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp > > b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp new file > > mode 100644 > > index 0000000000..5ba52cc069 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/frame-selection-last-instr-call.exp > > @@ -0,0 +1,130 @@ > > +# Copyright 2024 Free Software Foundation, Inc. >=20 > If we do keep this test, the copyright year should be kept as 2018-2024, > since the file is very close to an existing file. Ok, understood. > [...] > > +runto_main > > +gdb_breakpoint abort > > +gdb_continue_to_breakpoint abort > > + > > +gdb_test "bt" "#0 $hex in abort .*#1 $hex in frame_1 .*#2 $hex in > > main.*" "backtrace at breakpoint" > Similarly, if we keep this test, I would prefer if this backtrace test > used the "multi_line". That's because ".*" could eat whole lines, and > make the test pass when it shouldn't. Something like: >=20 > gdb_test "bt" [multi_line "#0 $hex in abort \[^\\r\\n\]*" "#1 $hex in > frame_1\[^\\r\\n\]*" "#2 $hex in main \[^\\r\\n\]*"] "backtrace at > breakpoint" >=20 > (with the appropriate line breaks, also I haven't tested the regexes, > they probably have some error in them). >=20 Got it. I'll modify this. Thanks, Sahil