Re: [RFA take 4] Allow setting breakpoints on inline functions (PR 10738)

[Gary Benson <gbenson at redhat dot com>]

Doug Evans wrote:
> On Tue, Feb 14, 2012 at 1:02 AM, Gary Benson <gbenson@redhat.com> wrote:
> > Doug Evans wrote:
> > > On Mon, Feb 13, 2012 at 10:47 AM, Gary Benson <gbenson@redhat.com> wrote:
> > > > This patch makes GDB able to set breakpoints on inlined
> > > > functions.
> > > >
> > > > This version of the patch has been updated to fix the issues
> > > > Jan pointed out with the last version.
> > > >
> > > > This patch bumps the version number of the .gdb-index to 6,
> > > > but it does not remove any of the backwards compatibility code
> > > > which I would prefer to do as a separate patch.
> > >
> > > I agree support for older versions should be a separate patch.
> > > However this patch doesn't do that (it removes current
> > > acceptance of older versions of the index).
> >
> > That's correct. ?The older versions do not contain partial symbols
> > for inlined functions. ?If GDB were to be run on a file with an
> > older versioned index without rejecting it then the ability to set
> > breakpoints on inlined functions would silently fail.
> 
> "silently fail" as in "work *worse* than it did in previous gdb
> versions"?

It wouldn't work *worse* than previous gdb versions--it would be the
same--but it would work worse than itself.  You would have the
situation where an optimization was causing functionality to be lost,
and that doesn't seem right to me.

> While it may seem like it's always a win to just discard the index
> to get new functionality, I'm not sure all users would agree with
> that in all situations.  If gdb 7.4 startup takes 5 seconds and gdb
> 7.5 startup takes 45 seconds on the same binary, and our response to
> their complaint is that they have to rebuild the index, I'm not sure
> I'd be comfortable with that.  Especially if, for example, they're,
> say, debugging a core file and can't use the new functionality
> anyway.  ***OTOH***, if there is a functional *regression* (as
> opposed to a speed regression) then I'd be much more comfortable
> with discarding the index.
> [to repeat my question above, for clarity's sake: Is there a
> functional regression if we don't discard the index?]
> OTOOH, 1/2 :-), why must there be a functional regression? [as
> opposed to the absence of a new feature or capability]

I guess it depends on how you define a functional regression.

If you compare GDB as it is now, GDB with this patch, and GDB with
this patch minus the index version, the functionality would look
like this:

                           |   GDB    GDB         GDB
                           |   now    ver bump    no ver bump
  -------------------------+-----------------------------------
                           |
  Break on inline funcs    |   no     yes         yes
  in files without index   |
                           |
  Break on inline funcs    |   no     yes         no
  in files with old index  |          (ignored)
     	   	    	   |
  Break on inline funcs	   |   no     yes         yes
  in files with new index  |


GDB's behaviour is inconsistent if you don't rejecting the old index
files.  That seemed like a bad thing to be introducing.

> One could support the old version for a release or two, and print a
> warning when older versions are encountered.

I wondered about this myself, though it was pointed out to me that
printed warnings often get lost in the noise.

> The user's build procedure may involve building the index in a way
> that is not easily updated in a timely manner.  Thus all the speed
> improvements are (at least temporarily, but for a long enough time to
> be troublesome) wiped out simply by using a *newer* version of gdb.
> And that makes me uncomfortable.

How would it be if there the default behaviour was be to reject old
indexes (as the patch does now) but with the addition of a flag
("maint set allow-old-gdb-indexes" perhaps?) that would allow users in
this particular situation to get around it?  That way, our response to
complaints can be "rebuild the index *or* use this flag (which by the
way will lose you such and such a functionality)"  Inconsistent
behaviour doesn't seem so bad if the user asked for it.

Cheers,
Gary

-- 
http://gbenson.net/