[Lldb-commits] [lldb] 419cc0a - [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via lldb-commits]

Author: Venkata Ramanaiah Nalamothu
Date: 2022-07-11T18:45:37+05:30
New Revision: 419cc0a0b2ab7306dd721c337e7ce6ed31dc7287

URL: https://github.com/llvm/llvm-project/commit/419cc0a0b2ab7306dd721c337e7ce6ed31dc7287
DIFF: https://github.com/llvm/llvm-project/commit/419cc0a0b2ab7306dd721c337e7ce6ed31dc7287.diff

LOG: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

The requirements for "thread until <line number>" are:

a) If any code contributed by <line number> or the nearest subsequent of <line number> is executed before leaving the function, stop
b) If you end up leaving the function w/o triggering (a), then stop

In case of (a), since the <line number> may have multiple entries in the line table and the compiler might have scheduled/moved the relevant code across, and the lldb does not know the control flow, set breakpoints on all the line table entries of best match of <line number> i.e. exact or the nearest subsequent line.

Along with the above, currently, CommandObjectThreadUntil is also setting the breakpoints on all the subsequent line numbers after the best match and this latter part is wrong.

This issue is discussed at http://lists.llvm.org/pipermail/lldb-dev/2018-August/013979.html.

In fact, currently `TestStepUntil.py` is not actually testing step until scenarios and `test_missing_one` test fails without this patch if tests are made to run. Fixed the test as well.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D50304

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectThread.cpp
    lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
    lldb/test/API/functionalities/thread/step_until/main.c

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index 993523e06736..9211480fa9f8 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@ class CommandObjectThreadUntil : public CommandObjectParsed {
         line_table->FindLineEntryByAddress(fun_end_addr, function_start,
                                            &end_ptr);
 
+        // Since not all source lines will contribute code, check if we are
+        // setting the breakpoint on the exact line number or the nearest
+        // subsequent line number and set breakpoints at all the line table
+        // entries of the chosen line number (exact or nearest subsequent).
         for (uint32_t line_number : line_numbers) {
+          LineEntry line_entry;
+          bool exact = false;
           uint32_t start_idx_ptr = index_ptr;
+          start_idx_ptr = sc.comp_unit->FindLineEntry(
+              index_ptr, line_number, nullptr, exact, &line_entry);
+          if (start_idx_ptr != UINT32_MAX)
+            line_number = line_entry.line;
+          exact = true;
+          start_idx_ptr = index_ptr;
           while (start_idx_ptr <= end_ptr) {
-            LineEntry line_entry;
-            const bool exact = false;
             start_idx_ptr = sc.comp_unit->FindLineEntry(
                 start_idx_ptr, line_number, nullptr, exact, &line_entry);
             if (start_idx_ptr == UINT32_MAX)

diff  --git a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
index 0145b34f31de..ee25d1343735 100644
--- a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@ def setUp(self):
         self.greater_than_two = line_number('main.c', 'Greater than or equal to 2.')
         self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-    def do_until (self, args, until_lines, expected_linenum):
+    def common_setup (self, args):
         self.build()
         exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@ def do_until (self, args, until_lines, expected_linenum):
         thread = threads[0]
         return thread
 
-        thread = self.common_setup(None)
+    def do_until (self, args, until_lines, expected_linenum):
+        thread = self.common_setup(args)
 
         cmd_interp = self.dbg.GetCommandInterpreter()
         ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@ def test_targetting_two_hitting_second (self):
         self.do_until(None, [self.less_than_two, self.greater_than_two], self.less_than_two)
 
     def test_missing_one (self):
-        """Test thread step until - targeting one line and missing it."""
+        """Test thread step until - targeting one line and missing it by stepping out to call site"""
         self.do_until(["foo", "bar", "baz"], [self.less_than_two], self.back_out_in_main)
 
 

diff  --git a/lldb/test/API/functionalities/thread/step_until/main.c b/lldb/test/API/functionalities/thread/step_until/main.c
index e0b4d8ab951e..bb866079cf5f 100644
--- a/lldb/test/API/functionalities/thread/step_until/main.c
+++ b/lldb/test/API/functionalities/thread/step_until/main.c
@@ -1,20 +1,25 @@
 #include <stdio.h>
 
-void call_me(int argc)
-{
+/* The return statements are purposefully so simple, and
+ * unrelated to the program, just to achieve consistent
+ * debug line tables, across platforms, that are not
+ * dependent on compiler optimzations. */
+int call_me(int argc) {
   printf ("At the start, argc: %d.\n", argc);
 
   if (argc < 2)
-    printf("Less than 2.\n");
+    return 1; /* Less than 2. */
   else
-    printf("Greater than or equal to 2.\n");
+    return argc; /* Greater than or equal to 2. */
 }
 
 int
 main(int argc, char **argv)
 {
-  call_me(argc);
-  printf("Back out in main.\n");
+  int res = 0;
+  res = call_me(argc); /* Back out in main. */
+  if (res)
+    printf("Result: %d. \n", res);
 
   return 0;
 }