[Lldb-commits] [lldb] r190149 - Correct logic error found by inspection.

[jingham at apple.com]

I must be dense today, but I can't see any logical difference between the version in your patch and the one that was there before.  

Jim

On Sep 6, 2013, at 2:03 PM, Malea, Daniel <daniel.malea at intel.com> wrote:

> Hi Ed/Jim,
> 
> After this fix, I'm noticing some buildbot failures in
> TestInlineStepping.py, which I reproduced locally:
> 
>  Process 25548 stopped
>  * thread #1: tid = 25548, 0x0000000000400721 a.out`main(argc=1,
> argv=0x00007fffd73b7b38) + 33 at calling.cpp:106, name = 'a.out', stop
> reason = step over
>  frame #0: 0x0000000000400721 a.out`main(argc=1, argv=0x00007fffd73b7b38)
> + 33 at calling.cpp:106
>  103 	    
>  104 	    inline_value = 0;    // Stop here and step over to set up
> stepping over.
>  105 	
> -> 106 	    inline_trivial_1 ();    // At inline_trivial_1 called from
> main.
>  107 	
>  108 	    caller_trivial_1();     // At first call of caller_trivial_1 in
> main.
>  109 	    
> (lldb) thread step-over
>  Process 25548 stopped
>  * thread #1: tid = 25548, 0x0000000000400728 a.out`main [inlined]
> inline_trivial_2() + 7 at calling.cpp:96, name = 'a.out', stop reason =
> step over
>  frame #0: 0x0000000000400728 a.out`main [inlined] inline_trivial_2() + 7
> at calling.cpp:96
>  93  	void
>  94  	inline_trivial_2 ()
>  95  	{
> -> 96  	    inline_value += 1; // In inline_trivial_2.
>  97  	    called_by_inline_trivial (); // At caller_by_inline_trivial in
> inline_trivial_2.
>  98  	}
>  99  	
> (lldb) 
> 
> 
> 
> It looks like the call to step-over the call on line 106 results in LLDB
> stopping two levels of inlined functions inside the call to
> inline_trivial_1(), rather than the line after the call to
> inline_trivial_1() as the user would expect..
> 
> Debugging the broken 'thread step-out' through the location in
> ThreadPlanStepOverRange.cpp that was modified in 190149, I noticed that
> while m_addr_context and older_context had equal comp_unit, function and
> symbol members, the block members were not equal. Due to the way the
> if-statement is nested, older_ctx_is_equivalent remains false and leads to
> the buggy behaviour. I have a potential fix (please see attached patch) --
> Jim when you have a moment, can you comment on the validity of the fix? I
> am not seeing any other regressions with my patch applied (with either
> clang trunk or gcc 4.7).
> 
> 
> Thanks,
> Dan
> 
> On 2013-09-06 8:43 AM, "Ed Maste" <emaste at freebsd.org> wrote:
> 
>> Author: emaste
>> Date: Fri Sep  6 07:43:14 2013
>> New Revision: 190149
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=190149&view=rev
>> Log:
>> Correct logic error found by inspection.
>> 
>> From Jim's post on the lldb-dev mailing list:
>> 
>> This code is there as a backstop for when the unwinder drops a frame at
>> the beginning of new function/trampoline or whatever.
>> 
>> In the (older_ctx_is_equivalent == false) case we will see if we are at
>> a trampoline function that somebody knows how to get out of, and
>> otherwise we will stop.
>> 
>> Modified:
>>   lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp
>> 
>> Modified: lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanSte
>> pOverRange.cpp?rev=190149&r1=190148&r2=190149&view=diff
>> ==========================================================================
>> ====
>> --- lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp (original)
>> +++ lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp Fri Sep  6
>> 07:43:14 2013
>> @@ -121,7 +121,7 @@ ThreadPlanStepOverRange::ShouldStop (Eve
>>            // in so I left out the target check.  And sometimes the
>> module comes in as the .o file from the
>>            // inlined range, so I left that out too...
>> 
>> -            bool older_ctx_is_equivalent = true;
>> +            bool older_ctx_is_equivalent = false;
>>            if (m_addr_context.comp_unit)
>>            {
>>                if (m_addr_context.comp_unit == older_context.comp_unit)
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> <older_ctx_equivalent_inline_stepping_fix.patch>