[lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in case of missing location at block begin

Thu Oct 11 16:44:10 PDT 2018

> On Oct 11, 2018, at 4:42 PM, Matthias Braun <matze at braunis.de> wrote:
> 
> I reverted things in r344318 now.
> 
>> On Oct 10, 2018, at 5:02 PM, Jim Ingham <jingham at apple.com> wrote:
>> 
>> Thanks for looking into this!  
>> 
>> When I was first working on inlined stepping, I found a bunch of cases where the line table info and the ranges for the inlined subroutines disagreed.  I remember seeing cases, for instance, where the address given for foo.h:xxx in the line table was contained in one of the debug_info's inlined subroutine blocks from a different file.  I tried for a little while to put in heuristics to try to work past the disagreement.  But that was too easy to get wrong, and when I got that wrong it often had the result of turning a step into a continue.  It is annoying to stop too early, but it is much worse to stop too late (or never).  So I ended up bagging my attempts at heroics and whenever I get to a place where the line table and the inlined_subroutine bits of info are out of sync, I just stop.
> Makes total sense from lldbs point of view. If anything this is something a verifier could catch during testing, but it's a small issue so I'm not sure it's worth starting this now.

Thanks for getting to the bottom of this!

-- adrian
> 
> - Matthias
> 
>> 
>> Jim
>> 
>> 
>>> On Oct 10, 2018, at 4:34 PM, Matthias Braun <matze at braunis.de> wrote:
>>> 
>>> 1) So I went and figured out why the lldb testcase at hand fails.
>>> 
>>> - It seems the debugger stepping logic will follow the program along until it finds a new source location. However if that new source location is part of a new DW_AT_abstract_location it is ignored in the step over mode.
>>> - In the testcase at hand the .loc location of an inlined function was moved to an earlier place without the DW_AT_abstract_location entry getting extended. So the debugger mistook the inlined function as the same scope and stepping incorrectly halted there.
>>> 
>>> On the LLVM side DW_AT_abstract_location is generated by LexicalScopes which is created by lib/CodeGen/LexicalScopes.cpp / extractLexicalScopes() with completely separate code from the line table generation code in lib/CodeGen/AsmPrinter/DwarfDebug.cpp you have to be aware of that and keep the two algorithms in sync :-/
>>> 
>>> I fixed LexicalScopes.cpp to be in sync and the lldb test works fine again for me.
>>> 
>>> 2) However talking to Adrian earlier he also expressed having a bad feeling about moving debug location upwards instead of emitting line-0 entries. So I think consensus here is to rather live with some line table bloat instead.
>>> 
>>> - Unless there are objections I will not go with option 1) but instead revert this commit tomorrow. Note that I will only revert r343874 (i.e. the behavior change for what to emit when the first instructions of a basic block have no location attached), but not r343895 (removing debuglocs from spill/reload instructions) as I still consider this a sensible commit. So in the end we may have bigger line tables than before my changes, but simpler code in the spill/reload generation and occasionally can avoid random source location when spill/reloads get rescheduled.
>>> 
>>> - Matthias
>>> 
>>> 
>>>> On Oct 10, 2018, at 1:17 PM, via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> -----Original Message-----
>>>>> From: Matthias Braun [mailto:matze at braunis.de]
>>>>> Sent: Wednesday, October 10, 2018 3:50 PM
>>>>> To: Robinson, Paul
>>>>> Cc: jingham at apple.com; vsk at apple.com; llvm-commits at lists.llvm.org; lldb-
>>>>> dev at lists.llvm.org
>>>>> Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in
>>>>> case of missing location at block begin
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Oct 10, 2018, at 12:18 PM, via llvm-commits <llvm-
>>>>> commits at lists.llvm.org> wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> -----Original Message-----
>>>>>>> From: jingham at apple.com [mailto:jingham at apple.com]
>>>>>>> Sent: Wednesday, October 10, 2018 2:20 PM
>>>>>>> To: Vedant Kumar
>>>>>>> Cc: Robinson, Paul; Vedant Kumar via llvm-commits; LLDB; Matthias Braun
>>>>>>> Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location
>>>>> in
>>>>>>> case of missing location at block begin
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>> On Oct 10, 2018, at 9:54 AM, Vedant Kumar <vsk at apple.com> wrote:
>>>>>>>> 
>>>>>>>>> On Oct 10, 2018, at 9:16 AM, Matthias Braun <matze at braunis.de> wrote:
>>>>>>>>> 
>>>>>>>>> So I haven't worked much on debug info, but here's the explanation
>>>>> for
>>>>>>> my patches:
>>>>>>>>> My original motivation was getting rid of code some code in the llvm
>>>>>>> codegen that for spills and reloads just picked the next debug location
>>>>>>> around. That just seemed wrong to me, as spills and reloads really are
>>>>>>> bookkeeping, we just move values around between registers and memory
>>>>> and
>>>>>>> none of it is related to anything the user wrote in the source program.
>>>>> So
>>>>>>> not assigning any debug information felt right for these instructions.
>>>>>>> Then people noticed line table bloat because of this and I guess we
>>>>>>> assumed that having exact line-0 annotations isn't that useful that it
>>>>>>> warrants bloating the debug information...
>>>>>>>> 
>>>>>>>> Right. This doesn't seem any more arbitrary than reusing the previous
>>>>>>> instruction location, which we do all the time. I think it's a
>>>>> reasonable
>>>>>>> tradeoff.
>>>>>> 
>>>>>> For spills and reloads, the next valid source location might be
>>>>> reasonable.
>>>>>> For top-of-block instructions, I really don't think so; we had this
>>>>> issue
>>>>>> in FastISel, some years back, and ultimately went with line-0 at top of
>>>>>> block because it caused way fewer problems than hoisting the next valid
>>>>>> source location.
>>>>> 
>>>>> I assume what happened in the past was that the previous debug location
>>>>> spilled over to the next basic block when the top-of-the-block instruction
>>>>> had line-0 set. I can immediately see why that is a problem. And I assume
>>>>> that was the case that was overlooked in the past. I cannot see however
>>>>> how taking the following location in the same basic block is a problem.
>>>> 
>>>> Because those instructions actually do something, and whether variables are
>>>> visible and expressions evaluate correctly may depend on those instructions
>>>> being executed before the debugger stops.  If you mark them with line 0 the
>>>> debugger doesn't stop. If you hoist the next source location, it does.
>>>> 
>>>> Spills and reloads in the middle of a block *probably* can be associated
>>>> with the next source line without doing any real damage; although it may
>>>> turn out that reloads need to attach to the previous source line.  I'd
>>>> need to see a variety of examples to be sure, one way or the other.
>>>> 
>>>> But long experience has taught me that instructions at the top of a block
>>>> really are better off at line 0 than at the next real source line.  If
>>>> you can prove they are *always* better off with the next source line, for
>>>> all consumers, I'd be very interested to hear about it.
>>>> 
>>>> But, this thread is not the best place for that; bring it up on llvm-dev
>>>> so other interested parties can see what you're looking for.
>>>> --paulr
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> 
>> 
> 