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

[Malea, Daniel]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: older_ctx_equivalent_inline_stepping_fix.patch
Type: application/octet-stream
Size: 1429 bytes
Desc: older_ctx_equivalent_inline_stepping_fix.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130906/5ea98a21/attachment.obj>