[lldb] Fix prologue size calculation for discontinuous functions #131597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When searching for the end of prologue, I'm only iterating through the address range (~basic block) which contains the function entry point. The reason for that is that even if some other range somehow contained the end-of-prologue marker, the fact that it's in a different range would imply it's reachable through some form of control flow, and that's usually not a good place to set an function entry breakpoint.
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesWhen searching for the end of prologue, I'm only iterating through the address range (~basic block) which contains the function entry point. The reason for that is that even if some other range somehow contained the end-of-prologue marker, the fact that it's in a different range would imply it's reachable through some form of control flow, and that's usually not a good place to set an function entry breakpoint. Full diff: https://github.com/llvm/llvm-project/pull/131597.diff 2 Files Affected:
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index c80f37ae68d9d..61e788da0e50b 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -662,10 +662,12 @@ uint32_t Function::GetPrologueByteSize() {
}
}
- const addr_t func_start_file_addr =
- m_range.GetBaseAddress().GetFileAddress();
- const addr_t func_end_file_addr =
- func_start_file_addr + m_range.GetByteSize();
+ AddressRange entry_range;
+ m_block.GetRangeContainingAddress(m_address, entry_range);
+ const addr_t range_start_file_addr = m_address.GetFileAddress();
+ const addr_t range_end_file_addr =
+ entry_range.GetBaseAddress().GetFileAddress() +
+ entry_range.GetByteSize();
// Now calculate the offset to pass the subsequent line 0 entries.
uint32_t first_non_zero_line = prologue_end_line_idx;
@@ -677,7 +679,7 @@ uint32_t Function::GetPrologueByteSize() {
break;
}
if (line_entry.range.GetBaseAddress().GetFileAddress() >=
- func_end_file_addr)
+ range_end_file_addr)
break;
first_non_zero_line++;
@@ -694,13 +696,13 @@ uint32_t Function::GetPrologueByteSize() {
// Verify that this prologue end file address in the function's address
// range just to be sure
- if (func_start_file_addr < prologue_end_file_addr &&
- prologue_end_file_addr < func_end_file_addr) {
- m_prologue_byte_size = prologue_end_file_addr - func_start_file_addr;
+ if (range_start_file_addr < prologue_end_file_addr &&
+ prologue_end_file_addr < range_end_file_addr) {
+ m_prologue_byte_size = prologue_end_file_addr - range_start_file_addr;
}
if (prologue_end_file_addr < line_zero_end_file_addr &&
- line_zero_end_file_addr < func_end_file_addr) {
+ line_zero_end_file_addr < range_end_file_addr) {
m_prologue_byte_size +=
line_zero_end_file_addr - prologue_end_file_addr;
}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
index 197ab9aa14910..06934c2bfe9c4 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
@@ -16,22 +16,32 @@ image lookup -v -n foo
# CHECK-LABEL: image lookup -v -n foo
# CHECK: 1 match found in {{.*}}
# CHECK: Summary: input.o`foo
-# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x000000000000000e)[0x0000000000000014-0x000000000000001c)
+# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x000000000000000f)[0x0000000000000015-0x000000000000001d)
image lookup -v --regex -n '^foo$'
# CHECK-LABEL: image lookup -v --regex -n '^foo$'
# CHECK: 1 match found in {{.*}}
# CHECK: Summary: input.o`foo
-# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x000000000000000e)[0x0000000000000014-0x000000000000001c)
+# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x000000000000000f)[0x0000000000000015-0x000000000000001d)
expr -- &foo
# CHECK-LABEL: expr -- &foo
# CHECK: (void (*)()) $0 = 0x0000000000000007
+breakpoint set --name foo --skip-prologue false
+# CHECK-LABEL: breakpoint set --name foo --skip-prologue false
+# CHECK: Breakpoint 1: where = input.o`foo at -:1, address = 0x0000000000000007
+
+breakpoint set --name foo --skip-prologue true
+# CHECK-LABEL: breakpoint set --name foo --skip-prologue true
+# CHECK: Breakpoint 2: where = input.o`foo + 1 at -:2, address = 0x0000000000000008
+
#--- input.s
.text
+ .file 0 "." "-"
foo.__part.1:
+ .loc 0 10
.cfi_startproc
callq bar
jmp foo.__part.3
@@ -41,7 +51,10 @@ foo.__part.1:
.type foo,@function
foo:
+ .loc 0 1
.cfi_startproc
+ nop
+ .loc 0 2 prologue_end
cmpl $0, %edi
je foo.__part.2
jmp foo.__part.1
@@ -51,6 +64,7 @@ foo:
bar:
.cfi_startproc
+ .loc 0 100
movl $47, %eax
retq
.cfi_endproc
@@ -58,6 +72,7 @@ bar:
.size bar, .Lbar_end-bar
foo.__part.2:
+ .loc 0 20
.cfi_startproc
callq baz
jmp foo.__part.3
@@ -66,6 +81,7 @@ foo.__part.2:
.cfi_endproc
foo.__part.3:
+ .loc 0 30
.cfi_startproc
retq
.Lfoo.__part.3_end:
@@ -87,6 +103,8 @@ foo.__part.3:
.byte 35 # DW_FORM_rnglistx
.byte 116 # DW_AT_rnglists_base
.byte 23 # DW_FORM_sec_offset
+ .byte 16 # DW_AT_stmt_list
+ .byte 23 # DW_FORM_sec_offset
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 2 # Abbreviation Code
@@ -127,6 +145,7 @@ foo.__part.3:
.quad 0 # DW_AT_low_pc
.byte 1 # DW_AT_ranges
.long .Lrnglists_table_base0 # DW_AT_rnglists_base
+ .long .Lline_table_start0 # DW_AT_stmt_list
.byte 2 # Abbrev [2] DW_TAG_subprogram
.quad bar # DW_AT_low_pc
.quad .Lbar_end # DW_AT_high_pc
@@ -183,3 +202,5 @@ foo.__part.3:
.Ldebug_list_header_end0:
.section ".note.GNU-stack","",@progbits
+ .section .debug_line,"",@progbits
+.Lline_table_start0:
|
JDevlieghere
approved these changes
Mar 19, 2025
lldb/source/Symbol/Function.cpp
Outdated
Comment on lines
667
to
670
| const addr_t range_start_file_addr = m_address.GetFileAddress(); | ||
| const addr_t range_end_file_addr = | ||
| entry_range.GetBaseAddress().GetFileAddress() + | ||
| entry_range.GetByteSize(); |
There was a problem hiding this comment.
Any reason to do it this way rather than:
const addr_t range_start_file_addr = entry_range.GetBaseAddress().GetFileAddress();
const addr_t range_end_file_addr = range_start_file_addr + entry_range.GetByteSize();
There was a problem hiding this comment.
Yes, because in the most general case, the entry point may not be at the start of any particular range (either because of a perverse compiler or because the linker placed two of the ranges next to each other and we have merged them into one).
I'm going to add a comment about that.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/6462 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When searching for the end of prologue, I'm only iterating through the address range (~basic block) which contains the function entry point. The reason for that is that even if some other range somehow contained the end-of-prologue marker, the fact that it's in a different range would imply it's reachable through some form of control flow, and that's usually not a good place to set an function entry breakpoint.