Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: Tom Tromey <tom@tromey.com>,
	gdb-patches@sourceware.org, Alexandre Oliva <oliva@gnu.org>
Subject: Re: [PATCH 2/2] gdb: Preserve is-stmt lines when switch between files
Date: Mon, 27 Apr 2020 11:34:18 +0100	[thread overview]
Message-ID: <20200427103418.GF3522@embecosm.com> (raw)
In-Reply-To: <AM6PR03MB51708748DE656914A5DA942DE4D10@AM6PR03MB5170.eurprd03.prod.outlook.com>

* Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-04-25 09:06:35 +0200]:

> CC Alexandre Oliva, who maintains this part of GCC.
> 
> 
> On 4/22/20 11:13 PM, Tom Tromey wrote:
> >>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> > 
> > I read through the various threads on this topic, though I think maybe
> > not very well.
> > 
> > IIUC, the issue at hand is that there are two proposed patches to fix
> > various issues in this area.
> > 
> > 
> > Andrew's patch takes us roughly to the status quo from before is-stmt
> > landed.  This patch takes the view that GCC is currently buggy, so one
> > test is partly disabled; and there's a new bug so we can fix GDB once
> > GCC is fixed.
> > 
> > Bernd's patch takes the view that GCC won't be fixed and tries to work
> > around the problem; it drops at least one "FILE:LINE" case that
> > currently works.
> > 
> > 
> > Is that a fair summary?  There were many messages and details, so I'm
> > not sure I either understood it all or that it can be summarized like
> > this.
> > 
> 
> Yes, I agree with this summary.
> 
> > If that's fair, then my inclination is to move forward with Andrew's
> > patch for the time being, with the primary justification being
> > conservatism.  Bernd's patch can always land if/when we have some
> > clarification from GCC about whether the bugs there will be fixed.
> > 
> > Let me know what you think.
> > 
> 
> I think if we take Andrew's patch the only possible outcome
> will be that gcc will disable that statement frontiers debug
> option by default, because they do not understand what we
> want from them.  (I am one of them, but I came here to understand
> you better)

I tried to describe the two current issues I see with GCC in this bug
report[1] but after a stupid mistake by me of using an older version
of binutils the conversation got side-tracked into a discussion about
view numbers and GDB.

It is my position that neither of the bugs described in [1] require
view number changes, nor additional DWARF features, they could be
fixed entirely withing GCC (in theory at least).

I'm not entirely sure which bit of debug statement frontiers is
responsible for.

After reading one of your other email[2], in which you said this bug
was present from GCC-7 I went and ran some tests, as I thought
gdb.cp/step-and-next-inline.exp doesn't run on GCC-7 as GCC that old
doesn't support -gstatement-frontiers.  I was right, that flag doesn't
exist on GCC-7, and your were correct that some of the range bugs (at
least) do still exist on GCC-7.

After reading[2] I'd also be interest to understand what flaw in DWARF
you feel makes a difference in this case.  Given some of your feedback
in [1] I think that you might be talking about improvements (the
addition of?) to the view number support, but while I agree with you
that this might (would?) allow for improved debug experience, I don't
think it would be needed in order for GCC to do better in this case.

I think it is great Bernd, that you are reaching out from the GCC
community to engage with GDB, this is certainly the best way to ensure
that we can work together as communities to give the best possible
debug experience, and I'm sorry you feel that I have not been clear
enough about the issues I'm seeing here.

I do still think that merging my patch is the correct way to move
forward, however, I don't think the future has to be as bleak as you
describe above.  I am happy to continue discussing this issue for as
long as you are in order to try and find a solution that makes
everyone happy.  That said, here's what I think would need to happen
so that _I_ (personally) was happy to accept your patch:

  1. We need to agree what we're working around.  In [2] you describe
  this issue as a flaw in DWARF, while in [1] I describe this as an
  issue in GCC.  I think we need to first need to agree on what the
  spec says, what GCC can (currently) do in an ideal world, and what
  GCC can be expected to do (currently) in the real world.

  2. We should change GDB assuming that any bugs in GCC or DWARF will
  one day be fixed, and that when that happens we would, ideally, not
  have to apply the work around.  So fixes should be guarded with a
  check of either the producer or the DWARF version.

  3. Currently (to me) it appears that we are crafting a work around
  based on one test case (gdb.cp/step-and-next-inline.exp).  My
  concern here is that when we talk about changing the line table, or
  range handling, these are pretty core aspects of GDB.

  As a GCC developer you probably have more insight, but it doesn't
  feel clear to me yet, do we know that GCC always misbehaves in the
  same say across all, or most, compiled code?

  I don't know how we address this without merging your patch,
  releasing GDB and seeing how it works in the wild.  However, if we
  did decide to "just try it", I would still prefer we staged things
  as:
    (a) Merge my patch, targeted regression fix, then
    (b) Your patch, new functionality GCC/DWARF ranges work around.
  In this way, if we end up backing out some or all of (b) we still
  have (a) in place that fixes the regression.  I'm more than happy
  for a rebase of (b) in include full removal of (a).

Thanks for your continued input.

Andrew

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474
[2] https://sourceware.org/pipermail/gdb-patches/2020-April/167933.html


  reply	other threads:[~2020-04-27 10:34 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05 11:37 [PATCH 0/2] Line table is_stmt support Andrew Burgess
2020-02-05 11:37 ` [PATCH 2/2] gdb: Add support for tracking the DWARF line table is-stmt field Andrew Burgess
2020-02-05 17:55   ` Bernd Edlinger
2020-02-10 18:30     ` Bernd Edlinger
2020-02-11 13:57     ` Andrew Burgess
2020-02-14 20:05       ` Bernd Edlinger
2020-03-05 18:01         ` Bernd Edlinger
2020-03-08 12:50           ` [PATCHv2 0/2] Line table is_stmt support Andrew Burgess
2020-03-08 14:39             ` Bernd Edlinger
2020-03-10 23:01               ` Andrew Burgess
2020-03-11  6:50                 ` Simon Marchi
2020-03-11 11:28                   ` Andrew Burgess
2020-03-11 13:27                     ` Simon Marchi
2020-04-03 22:21             ` [PATCH 0/2] More regression fixing from is-stmt patches Andrew Burgess
2020-04-03 22:21             ` [PATCH 1/2] gdb/testsuite: Move helper function into lib/dwarf.exp Andrew Burgess
2020-04-06 20:18               ` Tom Tromey
2020-04-14 11:18                 ` Andrew Burgess
2020-04-03 22:21             ` [PATCH 2/2] gdb: Preserve is-stmt lines when switch between files Andrew Burgess
2020-04-04 18:07               ` Bernd Edlinger
2020-04-04 19:59                 ` Bernd Edlinger
2020-04-04 22:23                 ` Andrew Burgess
2020-04-05  0:04                   ` Bernd Edlinger
2020-04-05  0:47                   ` Bernd Edlinger
2020-04-05  8:55                   ` Bernd Edlinger
2020-04-11  3:52               ` Bernd Edlinger
2020-04-12 17:13                 ` Bernd Edlinger
2020-04-14 11:28                   ` Andrew Burgess
2020-04-14 11:37                     ` Bernd Edlinger
2020-04-14 11:41                       ` Bernd Edlinger
2020-04-14 13:08                       ` Andrew Burgess
2020-04-16 17:18                     ` Andrew Burgess
2020-04-22 21:13                       ` Tom Tromey
2020-04-25  7:06                         ` Bernd Edlinger
2020-04-27 10:34                           ` Andrew Burgess [this message]
2020-05-14 20:18                             ` Tom Tromey
2020-05-14 22:39                               ` Andrew Burgess
2020-05-15  3:35                                 ` Bernd Edlinger
2020-05-15 14:46                                   ` Andrew Burgess
2020-05-16  8:12                                     ` Bernd Edlinger
2020-05-17 17:26                                   ` Bernd Edlinger
2020-05-20 18:26                                   ` Andrew Burgess
2020-05-27 13:10                                     ` Andrew Burgess
2020-06-01  9:05                                       ` Andrew Burgess
2020-03-08 12:50           ` [PATCHv2 1/2] gdb/testsuite: Add is-stmt support to the DWARF compiler Andrew Burgess
2020-03-08 12:50           ` [PATCHv2 2/2] gdb: Add support for tracking the DWARF line table is-stmt field Andrew Burgess
2020-03-16 20:57             ` Tom Tromey
2020-03-16 22:37               ` Bernd Edlinger
2020-03-17 12:47               ` Tom Tromey
2020-03-17 18:23                 ` Tom Tromey
2020-03-17 18:51                   ` Bernd Edlinger
2020-03-17 18:56                   ` Andrew Burgess
2020-03-17 20:18                     ` Tom Tromey
2020-03-17 22:21                       ` Andrew Burgess
2020-03-23 17:30             ` [PATCH 0/3] Keep duplicate line table entries Andrew Burgess
2020-03-23 17:30             ` [PATCH 1/3] gdb/testsuite: Add compiler options parameter to function_range helper Andrew Burgess
2020-04-01 18:31               ` Tom Tromey
2020-03-23 17:30             ` [PATCH 2/3] gdb/testsuite: Add support for DW_LNS_set_file to DWARF compiler Andrew Burgess
2020-04-01 18:32               ` Tom Tromey
2020-03-23 17:30             ` [PATCH 3/3] gdb: Don't remove duplicate entries from the line table Andrew Burgess
2020-04-01 18:34               ` Tom Tromey
2020-06-01 13:26           ` [PATCH 2/2] gdb: Add support for tracking the DWARF line table is-stmt field Pedro Alves
2020-02-06  9:01   ` Luis Machado
2020-02-11 15:39     ` Andrew Burgess
2020-02-09 21:07   ` [PATCH] Fix range end handling of inlined subroutines Bernd Edlinger
2020-02-10 21:48     ` Andrew Burgess
2020-02-22  6:39     ` [PATCHv2] " Bernd Edlinger
2020-03-08 14:57       ` [PATCHv3] " Bernd Edlinger
2020-03-11 22:02         ` Andrew Burgess
2020-03-12 18:21           ` Bernd Edlinger
2020-03-12 18:27             ` Christian Biesinger
2020-03-13  8:03               ` Bernd Edlinger
2020-03-17 22:27                 ` Andrew Burgess
2020-03-19  1:33                   ` Bernd Edlinger
2020-03-21 20:31                     ` Bernd Edlinger
2020-03-23 17:53                       ` Andrew Burgess
2020-03-23 20:58                         ` Bernd Edlinger
2020-06-01 14:28                           ` Pedro Alves
2020-03-13 12:47         ` [PATCHv4] " Bernd Edlinger
2020-02-05 11:37 ` [PATCH 1/2] gdb/testsuite: Add is-stmt support to the DWARF compiler Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200427103418.GF3522@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=gdb-patches@sourceware.org \
    --cc=oliva@gnu.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox