Author: Tatyana Krasnukha Date: 2020-03-16T16:20:11+03:00 New Revision: 0a840ef80059c761109f5988590a3616c00906a1
URL: https://github.com/llvm/llvm-project/commit/0a840ef80059c761109f5988590a3616c00906a1 DIFF: https://github.com/llvm/llvm-project/commit/0a840ef80059c761109f5988590a3616c00906a1.diff LOG: [lldb] Copy m_behaves_like_zeroth_frame on stack frame update Fix to code from https://reviews.llvm.org/D64993. Field StackFrame::m_behaves_like_zeroth_frame was introduced in commit [1], however that commit hasn't added a copying of the field to UpdatePreviousFrameFromCurrentFrame, therefore the value wouldn't change when updating frames to reflect the current situation. The particular scenario, where this matters is following. Assume we have function main that invokes function func1. We set breakpoint at func1 entry and in main after the func1 call, and do not stop at the main entry. Therefore, when debugger stops for the first time, func1 is frame#0, while main is frame#1, thus m_behaves_like_zeroth_frame is set to 0 for main frame. Execution is resumed, and stops now in main, where it is now frame#0. However while updating the frame object, m_behaves_like_zeroth_frame remains false. This field plays an important role when calculating line information for backtrace: for frame#0, PC is the current line, therefore line information is retrieved for PC, however for all other frames this is not the case - calculated PC is a return-PC, i.e. instruction after the function call line, therefore for those frames LLDB needs to step back by one instruction. Initial implementation did this strictly for frames that have index != 0 (and index is updated properly in UpdatePreviousFrameFromCurrentFrame), but m_behaves_like_zeroth_frame added a capability for middle-of-stack frames to behave in a similar manner. But because current code now doesn't check frame idx, m_behaves_like_zeroth_frame must be set to true for frames with 0 index, not only for frame that behave like one. In the described test case, after stopping in main, LLDB would still consider frame#0 as non-zeroth, and would subtract instruction from the PC, and would report previous like as current line. The error doesn't manifest itself in LLDB interpreter though - it can be reproduced through LLDB-MI and when using SB API, but not when we interpreter command "continue" is executed. Honestly, I didn't fully understand why it works in interpreter, I did found that bug "fixes" itself if I enable DEBUG_STACK_FRAMES in StackFrameList.cpp, because that calls StackFrame::Dump and that calls GetSymbolContext(eSymbolContextEverything), which fills the context of frame on the first breakpoint, therefore it doesn't have to be recalculated (improperly) on a second frame. However, on first breakpoint symbol context is calculated for the "call" line, not the next one, therefore it should be recalculated anyway on a second breakpoint, and it is done correctly, even though m_behaves_like_zeroth_frame is still incorrect, as long as GetSymbolContext(eSymbolContextEverything) has been called. [1] 31e6dbe1c6a6 Fix PC adjustment in StackFrame::GetSymbolContext Differential Revision: https://reviews.llvm.org/D75975 Patch by Anton Kolesov <anton.kole...@synopsys.com> Added: lldb/test/API/functionalities/unwind/zeroth_frame/Makefile lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py lldb/test/API/functionalities/unwind/zeroth_frame/main.c Modified: lldb/source/Target/StackFrame.cpp Removed: ################################################################################ diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index a81879476cf2..56761a6d1046 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -1861,6 +1861,7 @@ void StackFrame::UpdatePreviousFrameFromCurrentFrame(StackFrame &curr_frame) { m_concrete_frame_index = curr_frame.m_concrete_frame_index; m_reg_context_sp = curr_frame.m_reg_context_sp; m_frame_code_addr = curr_frame.m_frame_code_addr; + m_behaves_like_zeroth_frame = curr_frame.m_behaves_like_zeroth_frame; assert(!m_sc.target_sp || !curr_frame.m_sc.target_sp || m_sc.target_sp.get() == curr_frame.m_sc.target_sp.get()); assert(!m_sc.module_sp || !curr_frame.m_sc.module_sp || diff --git a/lldb/test/API/functionalities/unwind/zeroth_frame/Makefile b/lldb/test/API/functionalities/unwind/zeroth_frame/Makefile new file mode 100644 index 000000000000..15a931850e17 --- /dev/null +++ b/lldb/test/API/functionalities/unwind/zeroth_frame/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py new file mode 100644 index 000000000000..b85a92e4a92f --- /dev/null +++ b/lldb/test/API/functionalities/unwind/zeroth_frame/TestZerothFrame.py @@ -0,0 +1,96 @@ +""" +Test that line information is recalculated properly for a frame when it moves +from the middle of the backtrace to a zero index. + +This is a regression test for a StackFrame bug, where whether frame is zero or +not depends on an internal field. When LLDB was updating its frame list value +of the field wasn't copied into existing StackFrame instances, so those +StackFrame instances, would use an incorrect line entry evaluation logic in +situations if it was in the middle of the stack frame list (not zeroth), and +then moved to the top position. The diff erence in logic is that for zeroth +frames line entry is returned for program counter, while for other frame +(except for those that "behave like zeroth") it is for the instruction +preceding PC, as PC points to the next instruction after function call. When +the bug is present, when execution stops at the second breakpoint +SBFrame.GetLineEntry() returns line entry for the previous line, rather than +the one with a breakpoint. Note that this is specific to +SBFrame.GetLineEntry(), SBFrame.GetPCAddress().GetLineEntry() would return +correct entry. + +This bug doesn't reproduce through an LLDB interpretator, however it happens +when using API directly, for example in LLDB-MI. +""" + +from __future__ import print_function + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class ZerothFrame(TestBase): + mydir = TestBase.compute_mydir(__file__) + + def test(self): + """ + Test that line information is recalculated properly for a frame when it moves + from the middle of the backtrace to a zero index. + """ + self.build() + self.setTearDownCleanup() + + exe = self.getBuildArtifact("a.out") + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + bp1_line = line_number('main.c', '// Set breakpoint 1 here') + bp2_line = line_number('main.c', '// Set breakpoint 2 here') + + lldbutil.run_break_set_by_file_and_line( + self, + 'main.c', + bp1_line, + num_expected_locations=1) + lldbutil.run_break_set_by_file_and_line( + self, + 'main.c', + bp2_line, + num_expected_locations=1) + + process = target.LaunchSimple( + None, None, self.get_process_working_directory()) + self.assertTrue(process, VALID_PROCESS) + + thread = process.GetThreadAtIndex(0) + if self.TraceOn(): + print("Backtrace at the first breakpoint:") + for f in thread.frames: + print(f) + # Check that we have stopped at correct breakpoint. + self.assertEqual( + process.GetThreadAtIndex(0).frame[0].GetLineEntry().GetLine(), + bp1_line, + "LLDB reported incorrect line number.") + + # Important to use SBProcess::Continue() instead of + # self.runCmd('continue'), because the problem doesn't reproduce with + # 'continue' command. + process.Continue() + + thread = process.GetThreadAtIndex(0) + if self.TraceOn(): + print("Backtrace at the second breakpoint:") + for f in thread.frames: + print(f) + # Check that we have stopped at the breakpoint + self.assertEqual( + thread.frame[0].GetLineEntry().GetLine(), + bp2_line, + "LLDB reported incorrect line number.") + # Double-check with GetPCAddress() + self.assertEqual( + thread.frame[0].GetLineEntry().GetLine(), + thread.frame[0].GetPCAddress().GetLineEntry().GetLine(), + "LLDB reported incorrect line number.") diff --git a/lldb/test/API/functionalities/unwind/zeroth_frame/main.c b/lldb/test/API/functionalities/unwind/zeroth_frame/main.c new file mode 100644 index 000000000000..5886846e8358 --- /dev/null +++ b/lldb/test/API/functionalities/unwind/zeroth_frame/main.c @@ -0,0 +1,8 @@ +void func_inner() { + int a = 1; // Set breakpoint 1 here +} + +int main() { + func_inner(); + return 0; // Set breakpoint 2 here +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
