This revision was automatically updated to reflect the committed changes.
Closed by commit rL297817: BreakpointResolverFileLine: Restrict
move-to-nearest-code from moving across… (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D30817?vs=91693&id=91848#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30817
Files:
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
Index: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
===================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
@@ -63,6 +63,8 @@
lldb::BreakpointResolverSP CopyForBreakpoint(Breakpoint &breakpoint) override;
protected:
+ void FilterContexts(SymbolContextList &sc_list);
+
friend class Breakpoint;
FileSpec m_file_spec; // This is the file spec we are looking for.
uint32_t m_line_number; // This is the line number that we are looking for.
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
@@ -0,0 +1,6 @@
+LLDB_TEST_API inline int foo1() { return 1; } // !BR1
+
+LLDB_TEST_API inline int foo2() { return 2; } // !BR2
+
+LLDB_TEST_API extern int call_foo1();
+LLDB_TEST_API extern int call_foo2();
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
@@ -0,0 +1,9 @@
+#include "foo.h"
+
+int call_foo2() { return foo2(); }
+
+int
+main() // !BR_main
+{
+ return call_foo1() + call_foo2();
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
@@ -0,0 +1,3 @@
+#include "foo.h"
+
+int call_foo1() { return foo1(); }
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
@@ -0,0 +1,58 @@
+from __future__ import print_function
+
+
+import unittest2
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestMoveNearest(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+ # Find the line number to break inside main().
+ self.line1 = line_number('foo.h', '// !BR1')
+ self.line2 = line_number('foo.h', '// !BR2')
+ self.line_main = line_number('main.cpp', '// !BR_main')
+
+ def test(self):
+ """Test target.move-to-nearest logic"""
+
+ self.build()
+ target = self.dbg.CreateTarget("a.out")
+ self.assertTrue(target, VALID_TARGET)
+
+ # Regardless of the -m value the breakpoint should have exactly one
+ # location on the foo functions
+ self.runCmd("settings set target.move-to-nearest-code true")
+ lldbutil.run_break_set_by_file_and_line(self, 'foo.h', self.line1,
+ loc_exact=True, extra_options="-m 1")
+ lldbutil.run_break_set_by_file_and_line(self, 'foo.h', self.line2,
+ loc_exact=True, extra_options="-m 1")
+
+ self.runCmd("settings set target.move-to-nearest-code false")
+ lldbutil.run_break_set_by_file_and_line(self, 'foo.h', self.line1,
+ loc_exact=True, extra_options="-m 0")
+ lldbutil.run_break_set_by_file_and_line(self, 'foo.h', self.line2,
+ loc_exact=True, extra_options="-m 0")
+
+
+ # Make sure we set a breakpoint in main with -m 1 for various lines in
+ # the function declaration
+ # "int"
+ lldbutil.run_break_set_by_file_and_line(self, 'main.cpp',
+ self.line_main-1, extra_options="-m 1")
+ # "main()"
+ lldbutil.run_break_set_by_file_and_line(self, 'main.cpp',
+ self.line_main, extra_options="-m 1")
+ # "{"
+ lldbutil.run_break_set_by_file_and_line(self, 'main.cpp',
+ self.line_main+1, extra_options="-m 1")
+ # "return .."
+ lldbutil.run_break_set_by_file_and_line(self, 'main.cpp',
+ self.line_main+2, extra_options="-m 1")
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
@@ -0,0 +1,8 @@
+LEVEL = ../../../make
+
+DYLIB_NAME := foo
+DYLIB_CXX_SOURCES := foo.cpp
+CXX_SOURCES := main.cpp
+CFLAGS_EXTRAS += -fPIC
+
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
@@ -1,8 +1,4 @@
-// Set break point at this line.
-
extern "C" int foo(void);
-int
-main (int argc, char **argv)
-{
+int main (int argc, char **argv) { // Set break point at this line.
return foo();
}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
@@ -45,14 +45,6 @@
extra_options="-K 0",
num_expected_locations=1)
- # This should create a breakpoint 0 locations.
- lldbutil.run_break_set_by_file_and_line(
- self,
- "main.cpp",
- self.line,
- extra_options="-m 0",
- num_expected_locations=0)
-
# Run the program.
self.runCmd("run", RUN_SUCCEEDED)
@@ -68,8 +60,6 @@
"1: file = 'main.cpp', line = %d, exact_match = 0, locations = 1" %
self.line,
"2: file = 'main.cpp', line = %d, exact_match = 0, locations = 1" %
- self.line,
- "3: file = 'main.cpp', line = %d, exact_match = 1, locations = 0" %
self.line])
# Continue the program, there should be another stop.
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
@@ -226,64 +226,73 @@
self.runCmd(
"-interpreter-exec console \"settings set target.move-to-nearest-code off\"")
self.expect("\^done")
- line = line_number('main.cpp', '// BP_before_main')
- self.runCmd("-break-insert -f main.cpp:%d" % line)
+ line_decl = line_number('main.cpp', '// BP_main_decl')
+ line_in = line_number('main.cpp', '// BP_in_main')
+ self.runCmd("-break-insert -f main.cpp:%d" % line_in)
self.expect("\^done,bkpt={number=\"1\"")
# Test that non-pending BP will not be set on non-existing line if target.move-to-nearest-code=off
# Note: this increases the BP number by 1 even though BP #2 is invalid.
- self.runCmd("-break-insert main.cpp:%d" % line)
+ self.runCmd("-break-insert main.cpp:%d" % line_in)
self.expect(
"\^error,msg=\"Command 'break-insert'. Breakpoint location 'main.cpp:%d' not found\"" %
- line)
+ line_in)
# Set target.move-to-nearest-code=on and target.skip-prologue=on and
- # set BP #3
+ # set BP #3 & #4
self.runCmd(
"-interpreter-exec console \"settings set target.move-to-nearest-code on\"")
self.runCmd(
"-interpreter-exec console \"settings set target.skip-prologue on\"")
self.expect("\^done")
- self.runCmd("-break-insert main.cpp:%d" % line)
+ self.runCmd("-break-insert main.cpp:%d" % line_in)
self.expect("\^done,bkpt={number=\"3\"")
+ self.runCmd("-break-insert main.cpp:%d" % line_decl)
+ self.expect("\^done,bkpt={number=\"4\"")
- # Set target.skip-prologue=off and set BP #4
+ # Set target.skip-prologue=off and set BP #5
self.runCmd(
"-interpreter-exec console \"settings set target.skip-prologue off\"")
self.expect("\^done")
- self.runCmd("-break-insert main.cpp:%d" % line)
- self.expect("\^done,bkpt={number=\"4\"")
+ self.runCmd("-break-insert main.cpp:%d" % line_decl)
+ self.expect("\^done,bkpt={number=\"5\"")
- # Test that BP #4 is located before BP #3
+ # Test that BP #5 is located before BP #4
self.runCmd("-exec-run")
self.expect("\^running")
self.expect(
+ "\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"5\"")
+
+ # Test that BP #4 is hit
+ self.runCmd("-exec-continue")
+ self.expect("\^running")
+ self.expect(
"\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"4\"")
# Test that BP #3 is hit
self.runCmd("-exec-continue")
self.expect("\^running")
self.expect(
"\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"3\"")
- # Test that the target.language=pascal setting works and that BP #5 is
+ # Test that the target.language=pascal setting works and that BP #6 is
# NOT set
self.runCmd(
"-interpreter-exec console \"settings set target.language c\"")
self.expect("\^done")
self.runCmd("-break-insert ns.foo1")
self.expect("\^error")
- # Test that the target.language=c++ setting works and that BP #6 is hit
+ # Test that the target.language=c++ setting works and that BP #7 is hit
self.runCmd(
"-interpreter-exec console \"settings set target.language c++\"")
self.expect("\^done")
self.runCmd("-break-insert ns::foo1")
- self.expect("\^done,bkpt={number=\"6\"")
+ self.expect("\^done,bkpt={number=\"7\"")
self.runCmd("-exec-continue")
self.expect("\^running")
self.expect(
- "\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"6\"")
+ "\*stopped,reason=\"breakpoint-hit\",disp=\"del\",bkptno=\"7\"")
# Test that BP #1 and #2 weren't set by running to program exit
self.runCmd("-exec-continue")
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
===================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
@@ -15,13 +15,16 @@
int foo2(void) { printf("In foo2\n"); return 2; }
}
-// BP_before_main
-
int x;
-int
-main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) { // BP_main_decl
printf("Print a formatted string so that GCC does not optimize this printf call: %s\n", argv[0]);
+ // This is a long comment with no code inside
+ // This is a long comment with no code inside
+ // This is a long comment with no code inside
+ // BP_in_main
+ // This is a long comment with no code inside
+ // This is a long comment with no code inside
+ // This is a long comment with no code inside
x = ns::foo1() + ns::foo2();
return 0; // BP_return
}
Index: lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
===================================================================
--- lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -108,6 +108,68 @@
return WrapOptionsDict(options_dict_sp);
}
+// Filter the symbol context list to remove contexts where the line number was
+// moved into a new function. We do this conservatively, so if e.g. we cannot
+// resolve the function in the context (which can happen in case of
+// line-table-only debug info), we leave the context as is. The trickiest part
+// here is handling inlined functions -- in this case we need to make sure we
+// look at the declaration line of the inlined function, NOT the function it was
+// inlined into.
+void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
+ if (m_exact_match)
+ return; // Nothing to do. Contexts are precise.
+
+ Log * log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS);
+ for(uint32_t i = 0; i < sc_list.GetSize(); ++i) {
+ SymbolContext sc;
+ sc_list.GetContextAtIndex(i, sc);
+ if (! sc.block)
+ continue;
+
+ FileSpec file;
+ uint32_t line;
+ const Block *inline_block = sc.block->GetContainingInlinedBlock();
+ if (inline_block) {
+ const Declaration &inline_declaration = inline_block->GetInlinedFunctionInfo()->GetDeclaration();
+ if (!inline_declaration.IsValid())
+ continue;
+ file = inline_declaration.GetFile();
+ line = inline_declaration.GetLine();
+ } else if (sc.function)
+ sc.function->GetStartLineSourceInfo(file, line);
+ else
+ continue;
+
+ if (file != sc.line_entry.file) {
+ LLDB_LOG(log, "unexpected symbol context file {0}", sc.line_entry.file);
+ continue;
+ }
+
+ // Compare the requested line number with the line of the function
+ // declaration. In case of a function declared as:
+ //
+ // int
+ // foo()
+ // {
+ // ...
+ //
+ // the compiler will set the declaration line to the "foo" line, which is
+ // the reason why we have -1 here. This can fail in case of two inline
+ // functions defined back-to-back:
+ //
+ // inline int foo1() { ... }
+ // inline int foo2() { ... }
+ //
+ // but that's the best we can do for now.
+ const int decl_line_is_too_late_fudge = 1;
+ if (m_line_number < line - decl_line_is_too_late_fudge) {
+ LLDB_LOG(log, "removing symbol context at {0}:{1}", file, line);
+ sc_list.RemoveContextAtIndex(i);
+ --i;
+ }
+ }
+}
+
Searcher::CallbackReturn
BreakpointResolverFileLine::SearchCallback(SearchFilter &filter,
SymbolContext &context,
@@ -117,24 +179,20 @@
assert(m_breakpoint != NULL);
// There is a tricky bit here. You can have two compilation units that
- // #include the same file, and
- // in one of them the function at m_line_number is used (and so code and a
- // line entry for it is generated) but in the
- // other it isn't. If we considered the CU's independently, then in the
- // second inclusion, we'd move the breakpoint
- // to the next function that actually generated code in the header file. That
- // would end up being confusing.
- // So instead, we do the CU iterations by hand here, then scan through the
- // complete list of matches, and figure out
- // the closest line number match, and only set breakpoints on that match.
+ // #include the same file, and in one of them the function at m_line_number is
+ // used (and so code and a line entry for it is generated) but in the other it
+ // isn't. If we considered the CU's independently, then in the second
+ // inclusion, we'd move the breakpoint to the next function that actually
+ // generated code in the header file. That would end up being confusing. So
+ // instead, we do the CU iterations by hand here, then scan through the
+ // complete list of matches, and figure out the closest line number match, and
+ // only set breakpoints on that match.
// Note also that if file_spec only had a file name and not a directory, there
- // may be many different file spec's in
- // the resultant list. The closest line match for one will not be right for
- // some totally different file.
- // So we go through the match list and pull out the sets that have the same
- // file spec in their line_entry
- // and treat each set separately.
+ // may be many different file spec's in the resultant list. The closest line
+ // match for one will not be right for some totally different file. So we go
+ // through the match list and pull out the sets that have the same file spec
+ // in their line_entry and treat each set separately.
const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
for (size_t i = 0; i < num_comp_units; i++) {
@@ -146,6 +204,9 @@
sc_list);
}
}
+
+ FilterContexts(sc_list);
+
StreamString s;
s.Printf("for %s:%d ", m_file_spec.GetFilename().AsCString("<Unknown>"),
m_line_number);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
