https://github.com/labath updated https://github.com/llvm/llvm-project/pull/123622
>From 0a80b7a54b49de65758ab48acdb6d92f9b674d71 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Mon, 20 Jan 2025 15:03:14 +0100 Subject: [PATCH 1/4] [lldb] Fix "in function" detection in "thread until" The implementation has an optimization which detects the range of line table entries covered by the function and then only searches for a matching line between them. This optimization was interfering with the logic for detecting whether a line belongs to the function because the first call to FindLineEntry was made with exact=false, which meant that if the function did not contain any exact matches, we would just pick the closest line number in that range, even if it was very far away. This patch fixes that by first attempting an inexact search across the entire line table, and then use the (potentially inexact) result of that for searching within the function. This makes the optimization a less effective, but I don't think we can differentiate between a line that belongs to the function (but doesn't have any code) and a line outside the function without that. The patch also avoids the use of (deprecated) Function::GetAddressRange by iterating over the GetAddressRanges result to find the full range of line entries for the function. --- lldb/source/Commands/CommandObjectThread.cpp | 57 ++++++++++--------- .../thread/step_until/TestStepUntil.py | 38 ++++++++++++- 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 4e2c4c1126bc3f..829abb8c5839bb 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -959,7 +959,6 @@ class CommandObjectThreadUntil : public CommandObjectParsed { } LineEntry function_start; - uint32_t index_ptr = 0, end_ptr = UINT32_MAX; std::vector<addr_t> address_list; // Find the beginning & end index of the function, but first make @@ -970,19 +969,22 @@ class CommandObjectThreadUntil : public CommandObjectParsed { return; } - AddressRange fun_addr_range = sc.function->GetAddressRange(); - Address fun_start_addr = fun_addr_range.GetBaseAddress(); - line_table->FindLineEntryByAddress(fun_start_addr, function_start, - &index_ptr); - Address fun_end_addr(fun_start_addr.GetSection(), - fun_start_addr.GetOffset() + - fun_addr_range.GetByteSize()); + uint32_t lowest_func_idx = UINT32_MAX; + uint32_t highest_func_idx = 0; + for (AddressRange range : sc.function->GetAddressRanges()) { + uint32_t idx; + LineEntry unused; + Address addr = range.GetBaseAddress(); + if (line_table->FindLineEntryByAddress(addr, unused, &idx)) + lowest_func_idx = std::min(lowest_func_idx, idx); - bool all_in_function = true; + addr.Slide(range.GetByteSize()); + if (line_table->FindLineEntryByAddress(addr, unused, &idx)) + highest_func_idx = std::max(highest_func_idx, idx); + } - line_table->FindLineEntryByAddress(fun_end_addr, function_start, - &end_ptr); + bool found_something = false; // Since not all source lines will contribute code, check if we are // setting the breakpoint on the exact line number or the nearest @@ -991,14 +993,15 @@ class CommandObjectThreadUntil : public CommandObjectParsed { 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; + if (sc.comp_unit->FindLineEntry(0, line_number, nullptr, exact, + &line_entry) == UINT32_MAX) + continue; + + found_something = true; + line_number = line_entry.line; exact = true; - start_idx_ptr = index_ptr; - while (start_idx_ptr <= end_ptr) { + uint32_t start_idx_ptr = lowest_func_idx; + while (start_idx_ptr <= highest_func_idx) { start_idx_ptr = sc.comp_unit->FindLineEntry( start_idx_ptr, line_number, nullptr, exact, &line_entry); if (start_idx_ptr == UINT32_MAX) @@ -1007,29 +1010,29 @@ class CommandObjectThreadUntil : public CommandObjectParsed { addr_t address = line_entry.range.GetBaseAddress().GetLoadAddress(target); if (address != LLDB_INVALID_ADDRESS) { - if (fun_addr_range.ContainsLoadAddress(address, target)) + AddressRange unused; + if (sc.function->GetRangeContainingLoadAddress(address, *target, + unused)) address_list.push_back(address); - else - all_in_function = false; } start_idx_ptr++; } } for (lldb::addr_t address : m_options.m_until_addrs) { - if (fun_addr_range.ContainsLoadAddress(address, target)) + AddressRange unused; + if (sc.function->GetRangeContainingLoadAddress(address, *target, + unused)) address_list.push_back(address); - else - all_in_function = false; } if (address_list.empty()) { - if (all_in_function) + if (found_something) result.AppendErrorWithFormat( - "No line entries matching until target.\n"); + "Until target outside of the current function.\n"); else result.AppendErrorWithFormat( - "Until target outside of the current function.\n"); + "No line entries matching until target.\n"); return; } diff --git a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py index 1fbb404aeae58b..9e2588dc61c007 100644 --- a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py +++ b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py @@ -16,9 +16,18 @@ def setUp(self): self.less_than_two = line_number("main.c", "Less than 2") 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") + self.in_foo = line_number("main.c", "In foo") + + def _build_dict_for_discontinuity(self): + return dict( + CFLAGS_EXTRAS="-funique-basic-block-section-names " + + "-ffunction-sections -fbasic-block-sections=list=" + + self.getSourcePath("function.list"), + LD_EXTRAS="-Wl,--script=" + self.getSourcePath("symbol.order"), + ) - def common_setup(self, args): - self.build() + def _common_setup(self, build_dict, args): + self.build(dictionary=build_dict) exe = self.getBuildArtifact("a.out") target = self.dbg.CreateTarget(exe) @@ -45,7 +54,7 @@ def common_setup(self, args): return thread def do_until(self, args, until_lines, expected_linenum): - thread = self.common_setup(args) + thread = self._common_setup(None, args) cmd_interp = self.dbg.GetCommandInterpreter() ret_obj = lldb.SBCommandReturnObject() @@ -88,3 +97,26 @@ def test_missing_one(self): self.do_until( ["foo", "bar", "baz"], [self.less_than_two], self.back_out_in_main ) + + @no_debug_info_test + def test_bad_line(self): + """Test that we get an error if attempting to step outside the current + function""" + thread = self._common_setup(None, None) + self.expect(f"thread until {self.in_foo}", + substrs=["Until target outside of the current function"], + error=True) + + @no_debug_info_test + @skipIf(oslist=lldbplatformutil.getDarwinOSTriples() + ["windows"]) + @skipIf(archs=no_match(["x86_64", "aarch64"])) + def test_bad_line_discontinuous(self): + """Test that we get an error if attempting to step outside the current + function -- and the function is discontinuous""" + self.build(dictionary=self._build_dict_for_discontinuity()) + _, _, thread, _ = lldbutil.run_to_source_breakpoint( + self, "At the start", lldb.SBFileSpec(self.main_source) + ) + self.expect(f"thread until {self.in_foo}", + substrs=["Until target outside of the current function"], + error=True) >From dbecc09b173e8fc3a196617d90f4f3624df4b52a Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Mon, 20 Jan 2025 16:10:50 +0100 Subject: [PATCH 2/4] format --- lldb/source/Commands/CommandObjectThread.cpp | 1 - .../thread/step_until/TestStepUntil.py | 16 ++++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 829abb8c5839bb..e472359a31337e 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -969,7 +969,6 @@ class CommandObjectThreadUntil : public CommandObjectParsed { return; } - uint32_t lowest_func_idx = UINT32_MAX; uint32_t highest_func_idx = 0; for (AddressRange range : sc.function->GetAddressRanges()) { diff --git a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py index 9e2588dc61c007..965da02ed0f984 100644 --- a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py +++ b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py @@ -103,9 +103,11 @@ def test_bad_line(self): """Test that we get an error if attempting to step outside the current function""" thread = self._common_setup(None, None) - self.expect(f"thread until {self.in_foo}", - substrs=["Until target outside of the current function"], - error=True) + self.expect( + f"thread until {self.in_foo}", + substrs=["Until target outside of the current function"], + error=True, + ) @no_debug_info_test @skipIf(oslist=lldbplatformutil.getDarwinOSTriples() + ["windows"]) @@ -117,6 +119,8 @@ def test_bad_line_discontinuous(self): _, _, thread, _ = lldbutil.run_to_source_breakpoint( self, "At the start", lldb.SBFileSpec(self.main_source) ) - self.expect(f"thread until {self.in_foo}", - substrs=["Until target outside of the current function"], - error=True) + self.expect( + f"thread until {self.in_foo}", + substrs=["Until target outside of the current function"], + error=True, + ) >From e8d4455aca3a9d3943926a64cfe309057d8d4c62 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Mon, 20 Jan 2025 17:30:54 +0100 Subject: [PATCH 3/4] add -1 to ensure the address is within the bounds of the function --- lldb/source/Commands/CommandObjectThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index e472359a31337e..c733210aa59c60 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -978,7 +978,7 @@ class CommandObjectThreadUntil : public CommandObjectParsed { if (line_table->FindLineEntryByAddress(addr, unused, &idx)) lowest_func_idx = std::min(lowest_func_idx, idx); - addr.Slide(range.GetByteSize()); + addr.Slide(range.GetByteSize() - 1); if (line_table->FindLineEntryByAddress(addr, unused, &idx)) highest_func_idx = std::max(highest_func_idx, idx); } >From 97320a500fcebd1a0ec1c4d89c8925d4781de0c1 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Thu, 30 Jan 2025 14:45:57 +0100 Subject: [PATCH 4/4] different -1 fix --- lldb/source/Commands/CommandObjectThread.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index c733210aa59c60..6edc1885dc1fa2 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -978,9 +978,14 @@ class CommandObjectThreadUntil : public CommandObjectParsed { if (line_table->FindLineEntryByAddress(addr, unused, &idx)) lowest_func_idx = std::min(lowest_func_idx, idx); - addr.Slide(range.GetByteSize() - 1); - if (line_table->FindLineEntryByAddress(addr, unused, &idx)) + addr.Slide(range.GetByteSize()); + if (line_table->FindLineEntryByAddress(addr, unused, &idx)) { highest_func_idx = std::max(highest_func_idx, idx); + } else { + // No line entry after the current function. The function is the + // last in the file, so we can just search until the end. + highest_func_idx = UINT32_MAX; + } } bool found_something = false; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits