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  <bernd.edlinger@hotmail.de>

	* 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  <bernd.edlinger@hotmail.de>

	* gdb.cp/next-inline.exp: New file.
	* gdb.cp/next-inline.cc: New file.
	* gdb.cp/next-inline.h: New file.



Thanks
Bernd.