[Lldb-commits] [lldb] [lldb] Handle improperly nested blocks differently (PR #117725)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Mar 26 06:31:28 PDT 2025


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 <pavel at 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,"", at 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,"", at 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 <pavel at 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



More information about the lldb-commits mailing list