labath updated this revision to Diff 91693.
labath added a comment.
Herald added a subscriber: ki.stfu.
I've updated the code as suggested. For the "fudge factor" I chose one, so that
this at least works in the fairly common case where you put an empty line
between two tiny functions. The "breakpoint on return type" case should still
work as long as the retyrn type does not span multiple lines, which I think is a
good compromise.
This required some tweaks to existing tests, as two of them were actually
relying on the move-the-breakpoint-into-a-function behavior. I have removed the
corresponding check from TestBreakpointOptions, as the new test supersedes that,
and I have tweaked TestMiBreak to test the move-nearest functionality
differently.
Let me know what you think.
https://reviews.llvm.org/D30817
Files:
include/lldb/Breakpoint/BreakpointResolverFileLine.h
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
source/Breakpoint/BreakpointResolverFileLine.cpp
Index: source/Breakpoint/BreakpointResolverFileLine.cpp
===================================================================
--- source/Breakpoint/BreakpointResolverFileLine.cpp
+++ source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -108,6 +108,67 @@
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.
+ if (m_line_number < line - 1) {
+ 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 +178,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 +203,9 @@
sc_list);
}
}
+
+ FilterContexts(sc_list);
+
StreamString s;
s.Printf("for %s:%d ", m_file_spec.GetFilename().AsCString("<Unknown>"),
m_line_number);
Index: packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/main.cpp
+++ 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: packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
===================================================================
--- packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/main.cpp
===================================================================
--- /dev/null
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
===================================================================
--- /dev/null
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.cpp
@@ -0,0 +1,3 @@
+#include "foo.h"
+
+int call_foo1() { return foo1(); }
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/TestMoveNearest.py
===================================================================
--- /dev/null
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile
===================================================================
--- /dev/null
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/main.cpp
+++ 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: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++ 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: include/lldb/Breakpoint/BreakpointResolverFileLine.h
===================================================================
--- include/lldb/Breakpoint/BreakpointResolverFileLine.h
+++ 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.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
