https://github.com/labath updated https://github.com/llvm/llvm-project/pull/117725
>From a2fe723ccfce64c3ccef67bbc75deba050d8dcc2 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Tue, 26 Nov 2024 16:12:40 +0100 Subject: [PATCH 1/2] [lldb] Handle improperly nested blocks differently In 6c7f56192fa6e689ef14d32e43a785de5692e9c0 (Sep 2011) we added code to extend the range of the parent block if the child block is not contained within it. Currently, this code doesn't work for (at least) two reasons: - we don't re-sort the ranges of the parent block, which means that the lookup may not find the range we added (and possibly other ranges as well) - The main address lookup mechanism (SymbolFileDWARF::ResolveSymbolContext(Address)) performs the lookup using DWARF DIE information, which doesn't contain this extra range. The motivation for the change was bad compiler output. The commit message does not go into details, so it's hard to say if this has been fixed, but given that was 13 years ago, I think we can assume that to be the case. In fact, as far as I can tell (I haven't tried building an lldb this old) this code stopped working in ea3e7d5ccf4f00741e4b106978bd8dab5cece3a1 (~two weeks later), when we added the requirement for the ranges to be sorted. Given that this isn't working, and that not changing the ranges of other blocks has other nice properties (the ranges can be immutable after construction), I'm removing the parent range changing code. I'm adding a test case that makes sure we don't do something egregious (e.g., crash) in this case. Having a child block not be a subset of the parent block doesn't seem to cause problems now, but if that ever turns out to be an issue, we can always intersect the child range with that of the parent. I'm also upgrading this message to a proper warning so we can see if this ever occurs in practice, and simplifying it so it doesn't get in the way of understanding the function. --- lldb/source/Symbol/Block.cpp | 37 +------ .../DWARF/x86/improperly_nested_blocks.s | 100 ++++++++++++++++++ 2 files changed, 105 insertions(+), 32 deletions(-) create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp index 6ecc988d7a5a9..2626361e6b7fb 100644 --- a/lldb/source/Symbol/Block.cpp +++ b/lldb/source/Symbol/Block.cpp @@ -333,38 +333,11 @@ void Block::FinalizeRanges() { void Block::AddRange(const Range &range) { Block *parent_block = GetParent(); if (parent_block && !parent_block->Contains(range)) { - Log *log = GetLog(LLDBLog::Symbols); - if (log) { - ModuleSP module_sp(m_parent_scope.CalculateSymbolContextModule()); - Function *function = m_parent_scope.CalculateSymbolContextFunction(); - const addr_t function_file_addr = function->GetAddress().GetFileAddress(); - const addr_t block_start_addr = function_file_addr + range.GetRangeBase(); - const addr_t block_end_addr = function_file_addr + range.GetRangeEnd(); - Type *func_type = function->GetType(); - - const Declaration &func_decl = func_type->GetDeclaration(); - if (func_decl.GetLine()) { - LLDB_LOGF(log, - "warning: %s:%u block {0x%8.8" PRIx64 - "} has range[%u] [0x%" PRIx64 " - 0x%" PRIx64 - ") which is not contained in parent block {0x%8.8" PRIx64 - "} in function {0x%8.8" PRIx64 "} from %s", - func_decl.GetFile().GetPath().c_str(), func_decl.GetLine(), - GetID(), (uint32_t)m_ranges.GetSize(), block_start_addr, - block_end_addr, parent_block->GetID(), function->GetID(), - module_sp->GetFileSpec().GetPath().c_str()); - } else { - LLDB_LOGF(log, - "warning: block {0x%8.8" PRIx64 "} has range[%u] [0x%" PRIx64 - " - 0x%" PRIx64 - ") which is not contained in parent block {0x%8.8" PRIx64 - "} in function {0x%8.8" PRIx64 "} from %s", - GetID(), (uint32_t)m_ranges.GetSize(), block_start_addr, - block_end_addr, parent_block->GetID(), function->GetID(), - module_sp->GetFileSpec().GetPath().c_str()); - } - } - parent_block->AddRange(range); + m_parent_scope.CalculateSymbolContextModule()->ReportWarning( + "block {0:x} has a range [{1:x}, {2:x}) which is not contained in the " + "parent block {3:x}", + GetID(), range.GetRangeBase(), range.GetRangeEnd(), + parent_block->GetID()); } m_ranges.Append(range); } diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s new file mode 100644 index 0000000000000..af744385993f2 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s @@ -0,0 +1,100 @@ +## This test checks that lldb handles (corrupt?) debug info which has improperly +## nested blocks. The behavior here is not prescriptive. We only want to check +## that we do something "reasonable". + + +# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t +# RUN: %lldb %t -o "image lookup -v -s look_me_up1" \ +# RUN: -o "image lookup -v -s look_me_up2" -o exit 2>&1 | FileCheck %s + +# CHECK-LABEL: image lookup -v -s look_me_up1 +# CHECK: warning: {{.*}} block 0x55 has a range [0x2, 0x4) which is not contained in the parent block 0x44 +# CHECK: Function: id = {0x00000030}, name = "fn", range = [0x0000000000000000-0x0000000000000005) +# CHECK: Blocks: id = {0x00000030}, range = [0x00000000-0x00000005) +# CHECK-NEXT: id = {0x00000044}, range = [0x00000001-0x00000003) +# CHECK-NEXT: id = {0x00000055}, range = [0x00000002-0x00000004) +# CHECK-NEXT: Symbol: + +# CHECK-LABEL: image lookup -v -s look_me_up2 +# CHECK: Function: id = {0x00000030}, name = "fn", range = [0x0000000000000000-0x0000000000000005) +# CHECK: Blocks: id = {0x00000030}, range = [0x00000000-0x00000005) +# CHECK-NEXT: Symbol: + + .text + .p2align 12 +fn: + nop +.Lblock1_begin: + nop +.Lblock2_begin: +look_me_up1: + nop +.Lblock1_end: +look_me_up2: + nop +.Lblock2_end: + nop +.Lfn_end: + + + .section .debug_abbrev,"",@progbits + .byte 1 # Abbreviation Code + .byte 17 # DW_TAG_compile_unit + .byte 1 # DW_CHILDREN_yes + .byte 37 # DW_AT_producer + .byte 8 # DW_FORM_string + .byte 17 # DW_AT_low_pc + .byte 1 # DW_FORM_addr + .byte 18 # DW_AT_high_pc + .byte 1 # DW_FORM_addr + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 2 # Abbreviation Code + .byte 46 # DW_TAG_subprogram + .byte 1 # DW_CHILDREN_yes + .byte 17 # DW_AT_low_pc + .byte 1 # DW_FORM_addr + .byte 18 # DW_AT_high_pc + .byte 1 # DW_FORM_addr + .byte 3 # DW_AT_name + .byte 8 # DW_FORM_string + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 3 # Abbreviation Code + .byte 11 # DW_TAG_lexical_block + .byte 1 # DW_CHILDREN_yes + .byte 17 # DW_AT_low_pc + .byte 1 # DW_FORM_addr + .byte 18 # DW_AT_high_pc + .byte 1 # DW_FORM_addr + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 0 # EOM(3) + + .section .debug_info,"",@progbits +.Lcu_begin0: + .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .short 5 # DWARF version number + .byte 1 # DWARF Unit Type + .byte 8 # Address Size (in bytes) + .long .debug_abbrev # Offset Into Abbrev. Section + .byte 1 # Abbrev DW_TAG_compile_unit + .asciz "Hand-written DWARF" # DW_AT_producer + .quad fn # DW_AT_low_pc + .quad .Lfn_end # DW_AT_high_pc + .byte 2 # Abbrev DW_TAG_subprogram + .quad fn # DW_AT_low_pc + .quad .Lfn_end # DW_AT_high_pc + .asciz "fn" # DW_AT_name + .byte 3 # Abbrev DW_TAG_lexical_block + .quad .Lblock1_begin # DW_AT_low_pc + .quad .Lblock1_end # DW_AT_high_pc + .byte 3 # Abbrev DW_TAG_lexical_block + .quad .Lblock2_begin # DW_AT_low_pc + .quad .Lblock2_end # DW_AT_high_pc + .byte 0 # End Of Children Mark + .byte 0 # End Of Children Mark + .byte 0 # End Of Children Mark + .byte 0 # End Of Children Mark +.Ldebug_info_end0: >From 10ec514412a314bdcac64ead8bd041aafc178168 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Wed, 26 Mar 2025 14:30:27 +0100 Subject: [PATCH 2/2] address review comments --- lldb/source/Symbol/Block.cpp | 19 +++++++++++++++---- .../DWARF/x86/improperly_nested_blocks.s | 5 ++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp index 2626361e6b7fb..4e5e765b990c3 100644 --- a/lldb/source/Symbol/Block.cpp +++ b/lldb/source/Symbol/Block.cpp @@ -15,6 +15,8 @@ #include "lldb/Symbol/VariableList.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" +#include "llvm/Support/raw_ostream.h" #include <memory> @@ -333,11 +335,20 @@ void Block::FinalizeRanges() { void Block::AddRange(const Range &range) { Block *parent_block = GetParent(); if (parent_block && !parent_block->Contains(range)) { - m_parent_scope.CalculateSymbolContextModule()->ReportWarning( - "block {0:x} has a range [{1:x}, {2:x}) which is not contained in the " - "parent block {3:x}", - GetID(), range.GetRangeBase(), range.GetRangeEnd(), + addr_t base_addr = + CalculateSymbolContextFunction()->GetAddress().GetFileAddress(); + + StreamString warning; + warning.AsRawOstream() << llvm::formatv("block {0:x} has a range ", + GetID()); + DumpAddressRange(warning.AsRawOstream(), base_addr + range.GetRangeBase(), + base_addr + range.GetRangeEnd(), 4); + warning.AsRawOstream() << llvm::formatv( + " which is not contained in the parent block {0:x} whose ranges are ", parent_block->GetID()); + parent_block->DumpAddressRanges(&warning, base_addr); + m_parent_scope.CalculateSymbolContextModule()->ReportWarning( + warning.GetData()); } m_ranges.Append(range); } diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s index af744385993f2..26c7fabe787ba 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/improperly_nested_blocks.s @@ -8,7 +8,7 @@ # RUN: -o "image lookup -v -s look_me_up2" -o exit 2>&1 | FileCheck %s # CHECK-LABEL: image lookup -v -s look_me_up1 -# CHECK: warning: {{.*}} block 0x55 has a range [0x2, 0x4) which is not contained in the parent block 0x44 +# CHECK: warning: {{.*}} block 0x55 has a range [0x00000002-0x00000004) which is not contained in the parent block 0x44 whose ranges are [0x00000001-0x00000003) # CHECK: Function: id = {0x00000030}, name = "fn", range = [0x0000000000000000-0x0000000000000005) # CHECK: Blocks: id = {0x00000030}, range = [0x00000000-0x00000005) # CHECK-NEXT: id = {0x00000044}, range = [0x00000001-0x00000003) @@ -90,6 +90,9 @@ look_me_up2: .byte 3 # Abbrev DW_TAG_lexical_block .quad .Lblock1_begin # DW_AT_low_pc .quad .Lblock1_end # DW_AT_high_pc + # Incorrect DWARF here: block 2 is contained within block 1, but + # [block2_begin, block2_end) is not a subrange of + # [block1_begin, block1_end) .byte 3 # Abbrev DW_TAG_lexical_block .quad .Lblock2_begin # DW_AT_low_pc .quad .Lblock2_end # DW_AT_high_pc _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits