Allow breakpoints to be set on C++11 inline initializers

[jingham]

Summary

The inline initializers contribute code to the constructor(s). You will step onto them in the source view as you step through the constructor, for instance. But because their source lines are outside the function source range, lldb thought a breakpoint on the initializer line was crossing from one function to another, which file & line breakpoints don't allow. That meant if you tried to set a breakpoint on one of these lines it doesn't create any locations.

This patch fixes that by asserting that if the LineEntry in one of the SymbolContexts that the search produced exactly matches the file & line specifications in the breakpoint, it has to be a valid place to set the breakpoint, and we should just set it.

[dblaikie]

"But because their source lines are outside the function source range"

Not sure I understand that - the DWARF doesn't describe a function
source range, right? Only the line a function starts on. And a
function can have code from source lines in many files/offsets that
are unrelated to the function start line (LLVM in several places
#includes .def files into functions to stamp out tables, switches,
arrays, etc, for instance)

[jingham]

If you set a breakpoint on source lines with no line table entries, lldb slides the actual match forward to the nearest line table entry to the given line number. That's necessary for instance because if you lay out your function definitions over multiple lines they won't all get line table entries, but we don't want people to have to guess which of them was... Same is true of more complicated compound statements.

So people like that, but they really hate it when they have:

#ifdef SOMETHING_NOT_DEFINED

int foo() {

}

#else
int bar() {

}
#end

but with lots more junk so you can't see the ifdef's and then they set a breakpoint in the "int foo" part and the breakpoint gets moved to bar instead and then doesn't get hit. You might try to argue that they should have checked where the breakpoint actually landed before coming into your office to yell at you, but it's likely to leave you with fewer friends...

So I was trying to detect this case and not move the breakpoint if sliding it crossed over the function start boundary. That way you'd see that the breakpoint didn't work, and go figure out why.

The thinko in the original version was that we were still doing this when we DIDN'T have to slide the breakpoint, when we got an exact match in the line table. In that case, we shouldn't try to second guess the line table at all. That's the patch in this fix.

BTW, the check wouldn't have affected code from .defs files because I only do it if the original breakpoint specification and the "function start" are from the same source file. And we know about inlined blocks so inlining isn't going to fool us either.

Jim

lldb/source/Breakpoint/BreakpointResolverFileLine.cpp

for(uint32_t i = 0; i < sc_list.GetSize(); ++i) {
     // inline int foo1() { ... }
     // inline int foo2() { ... }
     //
     // but that's the best we can do for now.
     // One complication, if the line number returned from GetStartLineSourceInfo
     // is 0, then we can't do this calculation.  That can happen if
     // GetStartLineSourceInfo gets an error, or if the first line number in
     // the function really is 0 - which happens for some languages.
+    
+    // But only do this calculation if the line number we found in the SC
+    // was different from the one requested in the source file.  If we actually
+    // found an exact match it must be valid.
+    
+    if (m_line_number == sc.line_entry.line)
+      continue;
+
     const int decl_line_is_too_late_fudge = 1;
     if (line && 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;
     }
   }
 }

lldb/test/API/lang/cpp/break-on-initializers/Makefile

+C_SOURCES := main.c
+CXXFLAGS_EXTRAS := -std=c++11
+
+include Makefile.rules

lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py

+"""
+When using C++11 in place member initialization, show that we
+can set and hit breakpoints on initialization lines.  This is a
+little bit tricky because we try not to move file and line breakpoints 
+across function boundaries but these lines are outside the source range
+of the constructor.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class RenameThisSampleTestTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test_breakpoints_on_initializers(self):
+        """Show we can set breakpoints on initializers appearing both before
+           and after the constructor body, and hit them."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.cpp")
+        self.first_initializer_line = line_number("main.cpp", "Set the before constructor breakpoint here")
+        self.second_initializer_line = line_number("main.cpp", "Set the after constructor breakpoint here")
+        self.sample_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Set up your test case here. If your test doesn't need any set up then
+        # remove this method from your TestCase class.
+
+    def sample_test(self):
+        """You might use the test implementation in several ways, say so here."""
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   " Set a breakpoint here to get started", self.main_source_file)
+
+        # Now set breakpoints on the two initializer lines we found in the test startup:
+        bkpt1 = target.BreakpointCreateByLocation(self.main_source_file, self.first_initializer_line)
+        self.assertEqual(bkpt1.GetNumLocations(), 1)
+        bkpt2 = target.BreakpointCreateByLocation(self.main_source_file, self.second_initializer_line)
+        self.assertEqual(bkpt2.GetNumLocations(), 1)
+
+        # Now continue, we should stop at the two breakpoints above, first the one before, then
+        # the one after.
+        self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt1)), 1, "Hit first breakpoint")
+        self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt2)), 1, "Hit second breakpoint")
+        
+

lldb/test/API/lang/cpp/break-on-initializers/main.cpp

+#include <stdio.h>
+#include <vector>
+
+class Trivial {
+public:
+  Trivial(int input) : m_int(input) {}
+private:
+  int m_int;
+
+};
+
+class Foo {
+private:
+  Trivial m_trivial = Trivial(100); // Set the before constructor breakpoint here
+
+public:
+  Foo(int input) {
+    printf("I have been made!\n");
+  }
+
+private:
+  Trivial m_other_trivial = Trivial(200); // Set the after constructor breakpoint here
+};
+
+int
+main()
+{
+  Foo myFoo(10); // Set a breakpoint here to get started
+  return 0;
+}
+