From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117998 invoked by alias); 5 Feb 2020 17:55:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 117976 invoked by uid 89); 5 Feb 2020 17:55:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=Hx-incomingtopheadermarker:Count, Hx-incomingtopheadermarker:SizeAsReceived, HX-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg:sk:0000000, Hx-incomingtopheadermarker:sk:UpperCa X-HELO: EUR04-DB3-obe.outbound.protection.outlook.com Received: from mail-oln040092074086.outbound.protection.outlook.com (HELO EUR04-DB3-obe.outbound.protection.outlook.com) (40.92.74.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Feb 2020 17:55:41 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=S2wnC1xkaTx9zzJEDfb7j6KuSHutVMrQUAhxJaESbaXIGn+yFLm7qkqV7H6H/TBpQOPhcgkgtE3XeRdbiVXCY05JONeKr2230bZR+ioOwU3Jrq/nXpkNB+D1YyjG/UyA0FGEONsQ7FaykgdaO6+r9wv5FardSLjAVxhWNYzQj8KDyJGCXMLdmP4WKv4VxsaRNBY7FbOipaDO+gHYXw6MdJdHPtBeqCvJ13Pdkyo3sUztNkPFSJ3E33wqqNYaQqfw5EIAn3hv1F7UFmPXstiLxCPMPVXEG3YGMAtOrllAsR23fNFTbpci1NuCr6rbYGLsP+LRzQ55ChPoafFTywVABA== 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=eZ3yBoXwNou5o6JuRM3TWyLhcyroeyDJCVsiFh8sc+w=; b=d58oyDY7H00qqLT0gj+cif9Hb39e9xO1uHdsCl65+mFOvPsCEiVwKthajkbCj5uOSySjwblHBH42Op3878jVPe2V8magNYnj50S/SA2JK0MtgDM7IZMUFWyz9u+InC6oOPp+3PuUKwbuhBXrx4HA0h/VbEePbUWcwCytQqDXNihtauBdnVmnxf7fQ+LAqm92KwAQLG2lw0K5KUM8GtFpJYXmJ6h+5tCpdheAr1OY6e+FHgYObOjvIdy005BjRU0ZM1wJzqQZuAH0NaaFP+5mNOKvP8gYS3oAcq608jQWLckk0q2oLaZ20LreUunoRmJy1mHzYxeLq7dWaxqRBfNq+g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from DB3EUR04FT043.eop-eur04.prod.protection.outlook.com (10.152.24.54) by DB3EUR04HT225.eop-eur04.prod.protection.outlook.com (10.152.24.209) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2707.21; Wed, 5 Feb 2020 17:55:38 +0000 Received: from HE1PR0301MB2330.eurprd03.prod.outlook.com (10.152.24.55) by DB3EUR04FT043.mail.protection.outlook.com (10.152.25.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2707.21 via Frontend Transport; Wed, 5 Feb 2020 17:55:37 +0000 Received: from HE1PR0301MB2330.eurprd03.prod.outlook.com ([fe80::8c79:f74c:1f18:8ee2]) by HE1PR0301MB2330.eurprd03.prod.outlook.com ([fe80::8c79:f74c:1f18:8ee2%12]) with mapi id 15.20.2686.031; Wed, 5 Feb 2020 17:55:37 +0000 Received: from [192.168.1.101] (92.77.140.102) by FRYP281CA0011.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2686.29 via Frontend Transport; Wed, 5 Feb 2020 17:55:37 +0000 From: Bernd Edlinger To: Andrew Burgess , "gdb-patches@sourceware.org" Subject: Re: [PATCH 2/2] gdb: Add support for tracking the DWARF line table is-stmt field Date: Wed, 05 Feb 2020 17:55:00 -0000 Message-ID: References: <94e33268f64060fc887670f4ee5ed524050cbcc7.1580902412.git.andrew.burgess@embecosm.com> In-Reply-To: <94e33268f64060fc887670f4ee5ed524050cbcc7.1580902412.git.andrew.burgess@embecosm.com> x-microsoft-original-message-id: <35bbd56e-3a7c-7e6f-c03a-cf1caeb07a1a@hotmail.de> x-ms-exchange-antispam-messagedata: K4PvfAUfrizqt19nSP4SDgNByrEjjQ8ZbWvxw3HIEJuPtMsNu8qnK4lI9vKdFEgzXqBS9H3xQJ1dckS0uAJrgu1L130+YocQ944/rmJ3Uj58qN9LbU7bN4EA3QNsGGO3jFPrcTF/Md2VEJcFpzlmhA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="Windows-1252" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SW-Source: 2020-02/txt/msg00109.txt.bz2 On 2/5/20 12:37 PM, Andrew Burgess wrote: > This commit brings support for the DWARF line table is_stmt field to > GDB. The is_stmt field is used by the compiler when a single source > line is split into multiple assembler instructions, especially if the > assembler instructions are interleaved with instruction from other > source lines. >=20 > The compiler will set the is_stmt flag false from some instructions > from the source lines, these instructions are not a good place to > insert a breakpoint in order to stop at the source line. > Instructions which are marked with the is_stmt flag true are a good > place to insert a breakpoint for that source line. >=20 > Currently GDB ignores all instructions for which is_stmt is false. > This is fine in a lot of cases, however, there are some cases where > this means the debug experience is not as good as it could be. >=20 > Consider stopping at a random instruction, currently this instruction > will be attributed to the last line table entry before this point for > which is_stmt was true - as these are the only line table entries that > GDB tracks. This can easily be incorrect in code with even a low > level of optimisation. >=20 > With is_stmt tracking in place, when stopping at a random instruction > we now attribute the instruction back to the real source line, even > when is_stmt is false for that instruction in the line table. >=20 > When inserting breakpoints we still select line table entries for > which is_stmt is true, so the breakpoint placing behaviour should not > change. >=20 > When stepping though code (at the line level, not the instruction > level) we will still stop at instruction where is_stmt is true, I > think this is more likely to be the desired behaviour. >=20 > Instruction stepping is, of course, unchanged, stepping one > instruction at a time, but we should now report more accurate line > table information with each instruction step. >=20 > The original motivation for this work was a patch posted by Bernd > here: > https://sourceware.org/ml/gdb-patches/2019-11/msg00792.html >=20 > As part of that thread it was suggested that many issues would be > resolved if GDB supported line table views, this isn't something I've > attempted in this patch, though reading the spec, it seems like this > would be a useful feature to support in GDB in the future. The spec > is here: > http://dwarfstd.org/ShowIssue.php?issue=3D170427.1 >=20 > And Bernd gives a brief description of the benefits here: > https://sourceware.org/ml/gdb-patches/2020-01/msg00147.html >=20 > With that all said, I think that there is benefit to having proper > is_stmt support regardless of whether we have views support, so I > think we should consider getting this (or something like this) in > first, and then building view support on top of this. >=20 > The gdb.cp/next-inline.exp test is based off a test proposed by Bernd > Edlinger in message: > https://sourceware.org/ml/gdb-patches/2019-12/msg00842.html >=20 Thanks Andrew, I played a bit with debugging the gcc internals, and I think the debug-experience is good, the problem with step over are completely gone, also in the original context. Just a few comments on the patch follow below. > gdb/ChangeLog: >=20 > * buildsym-legacy.c (record_line): Pass extra parameter to > record_line. > * buildsym.c (buildsym_compunit::record_line): Take an extra > parameter, reduce duplication in the line table, and record the > is_stmt flag in the line table. > * buildsym.h (buildsym_compunit::record_line): Add extra > parameter. > * disasm.c (do_mixed_source_and_assembly_deprecated): Ignore > non-statement lines. > * dwarf2read.c (dwarf_record_line_1): Add extra parameter, pass > this to the symtab builder. > (dwarf_finish_line): Pass extra parameter to dwarf_record_line_1. > (lnp_state_machine::record_line): Pass a suitable is_stmt flag > through to dwarf_record_line_1. > * infrun.c (process_event_stop_test): When stepping, don't stop at > a non-statement instruction, and only refresh the step info when > we land in the middle of a line's range. did you mean, when we don't land in the middle of a line ? > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -330,7 +330,7 @@ frame_show_address (struct frame_info *frame, > return false; > } >=20=20 > - return get_frame_pc (frame) !=3D sal.pc; > + return get_frame_pc (frame) !=3D sal.pc || !sal.is_stmt; > } >=20=20 > /* See frame.h. */ > @@ -1036,6 +1036,7 @@ print_frame_info (const frame_print_options &fp_opt= s, > int location_print; > struct ui_out *uiout =3D current_uiout; >=20=20 > + > if (!current_uiout->is_mi_like_p () > && fp_opts.print_frame_info !=3D print_frame_info_auto) > { Is this white space change intentional? Thanks Bernd.