From patchwork Sat Dec 28 08:40:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 37112 Received: (qmail 27076 invoked by alias); 28 Dec 2019 08:40:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27058 invoked by uid 89); 28 Dec 2019 08:40:51 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=explanation, Today, referred, Sep X-HELO: EUR05-DB8-obe.outbound.protection.outlook.com Received: from mail-db8eur05olkn2030.outbound.protection.outlook.com (HELO EUR05-DB8-obe.outbound.protection.outlook.com) (40.92.89.30) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 28 Dec 2019 08:40:47 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FcyNt6p/KI56SVQK782jdvXB36sBNrkfM032Xn+zq+azmVl+UEAvXZ2lNoTIEASm7/G6irVgRi3jeI9Ogs55ySi/BiGaDn589MNvJosa90tuf1DAZUfqCfQCcD7+zptNjdK+W9+Ecu09FY6gXc9qs4jtFyI4KrrkG4z6q4iOL2713FS9+WIxQzIjI1eWsZ2tJaOPknFsWdWVtO8d2Y8o/olDK7LBFvZhSN1QREPTc0LSfEozWG0IbzzfhLfly8GPipfVQPEmsZhhMJi5kIYCuS43lwF3DLEHB9K5sa3lub15DpDb0dD80B8Ls04k7WvnLIczr1Pgd+Fo5max1Ym6gA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=EzEUv66dMVGZ1CT+IDu7dxGpM3vb5A/0LMXHF6RKAn4=; b=CXp3EF+48VJgK2rOQipXRYXMACBj84te6nbKFsEQQ1fX2AeIL8LW/351oKziFiQQtGDgwVMu5BwVnv1w4A7p2G2BVTj5T+mxobFDw+T39kUlJTPEZsq6ze4LFMTNr37+AgsJ9cPhhMk0gIVxOdnvBLgrJtHLTciET+77+pqzHpE2ltgj8PfXe9g6Mbr/4eyNlzFOpkokyC6dzII0K1uNCWSzqbtrOTjn2i/llgDFnJCrJqt6RcaGOonV+Tn3sCwg4/XEb2wIar4BM/pXjfqGVPfXxS6OLD6w87y6ZVFw4/D+kkGcayYXjEAyE9O4wZMHF62IMU3hRDcIY2/emv8ZcQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from DB8EUR05FT060.eop-eur05.prod.protection.outlook.com (10.233.238.57) by DB8EUR05HT021.eop-eur05.prod.protection.outlook.com (10.233.238.194) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11; Sat, 28 Dec 2019 08:40:44 +0000 Received: from AM0PR08MB3714.eurprd08.prod.outlook.com (10.233.238.58) by DB8EUR05FT060.mail.protection.outlook.com (10.233.238.218) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11 via Frontend Transport; Sat, 28 Dec 2019 08:40:44 +0000 Received: from AM0PR08MB3714.eurprd08.prod.outlook.com ([fe80::8dd1:fb18:6271:f769]) by AM0PR08MB3714.eurprd08.prod.outlook.com ([fe80::8dd1:fb18:6271:f769%7]) with mapi id 15.20.2581.007; Sat, 28 Dec 2019 08:40:44 +0000 Received: from [192.168.1.101] (146.60.252.106) by ZRAP278CA0003.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:10::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11 via Frontend Transport; Sat, 28 Dec 2019 08:40:43 +0000 From: Bernd Edlinger To: Simon Marchi , "gdb-patches@sourceware.org" CC: Tom Tromey Subject: Re: [PATCH] Fix an issue with the gdb step-over aka. "n" command Date: Sat, 28 Dec 2019 08:40:44 +0000 Message-ID: References: In-Reply-To: x-microsoft-original-message-id: x-ms-exchange-transport-forked: True MIME-Version: 1.0 On 12/20/19 7:13 AM, Simon Marchi wrote: > Hi Bernd, > > On 2019-12-19 5:53 p.m., Bernd Edlinger wrote: >> Does this explanation make sense? > > Yes. Well I think so, I have to admit this is a bit over my head, > there are a lot of pieces to put together to have a good understanding > of the problem. I just did a first read, I'll sleep on it and come > back to it later. > > Thanks for the small reproducer, this is extremely valuable. I think it > will be a good idea to integrate it as a test case in the test suite. > > In your patch to dwarf2read.c, I was a bit surprised to see: > > m_last_subfile != m_cu->get_builder ()->get_current_subfile () > > So your fix only works if the inlined subroutine comes from another file? If > I move the tree_check function in next-inline.cc, the fix no longer applies, > and we get the broken behavior. From your previous email, I understand that > this is expected. I guess that if both are in the same file, we can't detect > this situation using the same technique. > > I also read about location views, since that's what Alexandre referred to. It > sounds like it's a magic solution that will allow GDB to do the right thing in > this kind of situation. If that's indeed the case, then it might be good to start > exploring this path. I'd like to have a better understanding of how this will help > GDB make a smarter "next", and what kind of effort is needed to make GDB use it. My > understanding is that location views allow having an address mapped to multiple > source locations. For example, here's the problematic address in the next-inline > test case I've compiled: > > ./next-inline.h:[++] > next-inline.h 28 0x1175 x > next-inline.h 30 0x1175 1 x > > ./next-inline.cc:[++] > next-inline.cc 22 0x1175 2 > > Today, when I ask GDB "which source line does this address correspond to", it gives me > one answer. Does this mean that GDB will now say that 0x1175 corresponds to > > - next-inline.h:28 > - next-inline.h:30 > - next-inline.cc:22 > > all at the same time? Is one of these source locations more important than the others? > If execution happens to stop exactly at this address, which location do we present to > the user? > > And to come back the problem at hand, how does this help GDB make a smarter "next"? > > Btw, I stumbled on a bug with the TUI source display. It might be caused by this patch, > or it might be that this patch uncovers it. > > When I do these actions: > > - Start GDB with the next-inline test file (from this patch) > - Enable the TUI > - Type "start" > - Type "s" > - Type "n" twice > > The TUI source display wrongfully jumps to the header file, line 24. > When I type "frame", it says I'm stopped at next-line.cc:24. So it > is showing the right line number of the wrong file. > I think meanwhile the display bug in the TUI window is fixed (by one of Tom Tromey's recent patches), could you please check again? I post both parts of the patch again for your reference here, and add a ChangeLog which I forgot previously: gdb: 2019-12-28 Bernd Edlinger * dwarf2read.c (lnp_state_machine): New member m_last_address. (lnp_state_machine::record_line): After a file changes or end sequence always assume a statement boundary. gdb/testsuite: 2019-12-28 Bernd Edlinger * gdb.cp/next-inline.exp: New file. * gdb.cp/next-inline.cc: New file. * gdb.cp/next-inline.h: New file. Thanks Bernd. From 95ba4780488b53104ea51cc0702f99a9a800984b Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Thu, 19 Dec 2019 23:41:37 +0100 Subject: [PATCH 2/2] Add a test case for step over inline functions --- gdb/testsuite/gdb.cp/next-inline.cc | 34 +++++++++++++++++++++++++++++++ gdb/testsuite/gdb.cp/next-inline.exp | 39 ++++++++++++++++++++++++++++++++++++ gdb/testsuite/gdb.cp/next-inline.h | 34 +++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 gdb/testsuite/gdb.cp/next-inline.cc create mode 100644 gdb/testsuite/gdb.cp/next-inline.exp create mode 100644 gdb/testsuite/gdb.cp/next-inline.h diff --git a/gdb/testsuite/gdb.cp/next-inline.cc b/gdb/testsuite/gdb.cp/next-inline.cc new file mode 100644 index 0000000..dcf5ab9 --- /dev/null +++ b/gdb/testsuite/gdb.cp/next-inline.cc @@ -0,0 +1,34 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2019 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include "next-inline.h" + +int __attribute__((noinline, noclone)) +get_alias_set (tree *t) +{ if (t != NULL + && TREE_TYPE (t).z != 1 + && TREE_TYPE (t).z != 2 + && TREE_TYPE (t).z != 3) + return 0; + return 1; +} + +tree xx; +int main() +{ get_alias_set(&xx); + abort(); +} diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp new file mode 100644 index 0000000..6badc8c --- /dev/null +++ b/gdb/testsuite/gdb.cp/next-inline.exp @@ -0,0 +1,39 @@ +# Copyright 2019 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +standard_testfile + +if { [prepare_for_testing "failed to prepare" "next-inline" \ + {next-inline.cc} \ + {debug nowarnings optimize=-O2}] } { + return -1 +} + +if ![runto_main] { + fail "can't run to main" + return +} + +gdb_test "bt" "\\s*\\#0\\s+main.*" "in main" +gdb_test "step" ".*" "step into get_alias_set" +gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline" +gdb_test "next" ".*" "next step 1" +gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 1" +gdb_test "next" ".*" "next step 2" +gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 2" +gdb_test "next" ".*" "next step 3" +gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 3" +gdb_test "next" ".*" "next step 4" +gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 4" diff --git a/gdb/testsuite/gdb.cp/next-inline.h b/gdb/testsuite/gdb.cp/next-inline.h new file mode 100644 index 0000000..99fb1b2 --- /dev/null +++ b/gdb/testsuite/gdb.cp/next-inline.h @@ -0,0 +1,34 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2019 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +struct tree{ + volatile int x; + volatile int z; +}; + +#define TREE_TYPE(NODE) (*tree_check (NODE, 0)) + +inline tree * +tree_check (tree *t, int i) +{ + if (t->x != i) + abort(); + tree *x = t; + return x; +} -- 1.9.1