[lldb][DWARF5] Enable macro evaluation

[kpdev42]

Summary

Patch enables handing of DWARFv5 DW_MACRO_define_strx and DW_MACRO_undef_strx

~~~

OS Laboratory. Huawei RRI. Saint-Petersburg

[tschuett]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h

27

If you would use a class instead of the struct, you could hide the member variables.

The class variant is slightly more verbose than the struct.

[clayborg]

lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp

95–97

This enum should be different otherwise this code change does nothing.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp

22

I would make this DWARF64 compatible.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h

28

"cu_offset" sounds like it is actually the offset of the compile unit. This is actually the compile unit offset for the string data for the compile unit in the string index section right? Maybe a name like "cu_str_offset" or "cu_strx_offset" would be a better name?

29

Can "data" ever be NULL? If not, then change this to a reference and add a constructor that inits the reference?

31

if you are going to keep "data" as a pointer, shouldn't this function also check "data" against nullptr?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1239–1244

If we want to change "data" to be a reference, we will need a constructor:

DWARFStrOffsetsInfo str_offsets_info(dwarf_cu->GetStrOffsetsBase(), symfile.GetDWARFContext().getOrLoadStrOffsetsData());

[clayborg]

lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp

96

Never mind me! Just checked the LoadOrGetSection definition. They were all different for other sections, so I thought they needed to be different...

[kpdev42]

Address review notes

[kpdev42]

Ping

[aprantl]

This test fails to compile on Darwin

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46816/testReport/junit/lldb-api/commands_expression_macros/TestMacros_py/
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46811/

Can you please add a @skipIfDarwin decorator?

[aprantl]

Out of curiosity — did you get an email notification from the bot?

[kpdev42]

In D130062#3789671, @aprantl wrote:

Out of curiosity — did you get an email notification from the bot?

Hi, yes, I will add a priority to these notifications to react faster, sorry

[aprantl]

In D130062#3789858, @kpdev42 wrote:

In D130062#3789671, @aprantl wrote:

Out of curiosity — did you get an email notification from the bot?

Hi, yes, I will add a priority to these notifications to react faster, sorry

Great. I was asking because we had issues with the bot not sending these out recently.

lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp

 
 const DWARFDataExtractor &DWARFContext::getOrLoadLocListsData() {
   return LoadOrGetSection(eSectionTypeDWARFDebugLocLists,
                           eSectionTypeDWARFDebugLocListsDwo,
                           m_data_debug_loclists);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadMacroData() {
-  return LoadOrGetSection(eSectionTypeDWARFDebugMacro, llvm::None,
-                          m_data_debug_macro);
+  return LoadOrGetSection(eSectionTypeDWARFDebugMacro,
+                          eSectionTypeDWARFDebugMacro, m_data_debug_macro);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadRangesData() {
   return LoadOrGetSection(eSectionTypeDWARFDebugRanges, llvm::None,
                           m_data_debug_ranges);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadRngListsData() {
   return LoadOrGetSection(eSectionTypeDWARFDebugRngLists,

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h

 namespace lldb_private {
 
 class DWARFDataExtractor;
 
 } // namespace lldb_private
 
 class SymbolFileDWARF;
 
+class DWARFStrOffsetsInfo {
+  lldb::offset_t cu_str_offset;
+  const lldb_private::DWARFDataExtractor &data;
+
+public:
+  DWARFStrOffsetsInfo(lldb::offset_t cu_str_offset,
+                      const lldb_private::DWARFDataExtractor &data)
+      : cu_str_offset(cu_str_offset), data(data) {}
+  uint64_t GetOffset(uint64_t index) const;
+};
+
 class DWARFDebugMacroHeader {
 public:
   enum HeaderFlagMask {
     OFFSET_SIZE_MASK = 0x1,
     DEBUG_LINE_OFFSET_MASK = 0x2,
     OPCODE_OPERANDS_TABLE_MASK = 0x4
   };
 
private:
   uint64_t m_debug_line_offset = 0;
 };
 
 class DWARFDebugMacroEntry {
 public:
   static void
   ReadMacroEntries(const lldb_private::DWARFDataExtractor &debug_macro_data,
                    const lldb_private::DWARFDataExtractor &debug_str_data,
+                   const DWARFStrOffsetsInfo *str_offsets_info,
                    const bool offset_is_64_bit, lldb::offset_t *sect_offset,
                    SymbolFileDWARF *sym_file_dwarf,
                    lldb_private::DebugMacrosSP &debug_macros_sp);
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGMACRO_H

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp

 
 #include "lldb/Symbol/DebugMacros.h"
 
 #include "DWARFDataExtractor.h"
 
 using namespace lldb_private;
 using namespace lldb_private::dwarf;
 
+uint64_t DWARFStrOffsetsInfo::GetOffset(uint64_t index) const {
+  uint64_t offset = cu_str_offset + data.GetDWARFSizeOfOffset() * index;
+  return data.GetU32(&offset);
+}
+
 DWARFDebugMacroHeader
 DWARFDebugMacroHeader::ParseHeader(const DWARFDataExtractor &debug_macro_data,
                                    lldb::offset_t *offset) {
   DWARFDebugMacroHeader header;
 
   // Skip over the version field in header.
   header.m_version = debug_macro_data.GetU16(offset);
 
for (uint64_t j = 0; j < operand_count; j++) {
       // Skip over the operand form
       debug_macro_data.GetU8(offset);
     }
   }
 }
 
 void DWARFDebugMacroEntry::ReadMacroEntries(
     const DWARFDataExtractor &debug_macro_data,
-    const DWARFDataExtractor &debug_str_data, const bool offset_is_64_bit,
+    const DWARFDataExtractor &debug_str_data,
+    const DWARFStrOffsetsInfo *str_offsets_info, const bool offset_is_64_bit,
     lldb::offset_t *offset, SymbolFileDWARF *sym_file_dwarf,
     DebugMacrosSP &debug_macros_sp) {
   llvm::dwarf::MacroEntryType type =
       static_cast<llvm::dwarf::MacroEntryType>(debug_macro_data.GetU8(offset));
   while (type != 0) {
     lldb::offset_t new_offset = 0, str_offset = 0;
     uint32_t line = 0;
     const char *macro_str = nullptr;
case DW_MACRO_undef_strp:
       macro_str = debug_str_data.GetCStr(&str_offset);
       if (type == DW_MACRO_define_strp)
         debug_macros_sp->AddMacroEntry(
             DebugMacroEntry::CreateDefineEntry(line, macro_str));
       else
         debug_macros_sp->AddMacroEntry(
             DebugMacroEntry::CreateUndefEntry(line, macro_str));
       break;
+    case DW_MACRO_define_strx:
+    case DW_MACRO_undef_strx:
+      line = debug_macro_data.GetULEB128(offset);
+      str_offset = debug_macro_data.GetULEB128(offset);
+      if (!str_offsets_info)
+        // Can't do much in this case, skip all such entries
+        continue;
+      str_offset = str_offsets_info->GetOffset(str_offset);
+      macro_str = debug_str_data.GetCStr(&str_offset);
+      if (type == DW_MACRO_define_strx)
+        debug_macros_sp->AddMacroEntry(
+            DebugMacroEntry::CreateDefineEntry(line, macro_str));
+      else
+        debug_macros_sp->AddMacroEntry(
+            DebugMacroEntry::CreateUndefEntry(line, macro_str));
+      break;
     case DW_MACRO_start_file:
       line = debug_macro_data.GetULEB128(offset);
       debug_line_file_idx = debug_macro_data.GetULEB128(offset);
       debug_macros_sp->AddMacroEntry(
           DebugMacroEntry::CreateStartFileEntry(line, debug_line_file_idx));
       break;
     case DW_MACRO_end_file:
       // This operation has no operands.
       debug_macros_sp->AddMacroEntry(DebugMacroEntry::CreateEndFileEntry());
       break;
     case DW_MACRO_import:
       if (offset_is_64_bit)
         new_offset = debug_macro_data.GetU64(offset);
       else
         new_offset = debug_macro_data.GetU32(offset);
       debug_macros_sp->AddMacroEntry(DebugMacroEntry::CreateIndirectEntry(
-          sym_file_dwarf->ParseDebugMacros(&new_offset)));
+          sym_file_dwarf->ParseDebugMacros(&new_offset, str_offsets_info)));
       break;
     default:
       // TODO: Add support for other standard operations.
       // TODO: Provide mechanism to hook handling of non-standard/extension
       // operands.
       return;
     }
     type = static_cast<llvm::dwarf::MacroEntryType>(
         debug_macro_data.GetU8(offset));
   }
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

 class DWARFDebugRanges;
 class DWARFDeclContext;
 class DWARFFormValue;
 class DWARFTypeUnit;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
 
+class DWARFStrOffsetsInfo;
+
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
 class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
                         public lldb_private::UserID {
   /// LLVM RTTI support.
   static char ID;
 
 public:
public:
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit &dwarf_cu);
 
   virtual void GetObjCMethods(lldb_private::ConstString class_name,
                               llvm::function_ref<bool(DWARFDIE die)> callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
-  lldb_private::DebugMacrosSP ParseDebugMacros(lldb::offset_t *offset);
+  lldb_private::DebugMacrosSP
+  ParseDebugMacros(lldb::offset_t *offset,
+                   const DWARFStrOffsetsInfo *str_offsets_info);
 
   static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
 
   lldb::ModuleSP GetExternalModule(lldb_private::ConstString name);
 
   typedef std::map<lldb_private::ConstString, lldb::ModuleSP>
       ExternalTypeModuleMap;
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
   } else {
     comp_unit.SetLineTable(line_table_up.release());
   }
 
   return true;
 }
 
 lldb_private::DebugMacrosSP
-SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset) {
+SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset,
+                                  const DWARFStrOffsetsInfo *str_offsets_info) {
   auto iter = m_debug_macros_map.find(*offset);
   if (iter != m_debug_macros_map.end())
     return iter->second;
 
   ElapsedTime elapsed(m_parse_time);
   const DWARFDataExtractor &debug_macro_data = m_context.getOrLoadMacroData();
   if (debug_macro_data.GetByteSize() == 0)
     return DebugMacrosSP();
 
   lldb_private::DebugMacrosSP debug_macros_sp(new lldb_private::DebugMacros());
   m_debug_macros_map[*offset] = debug_macros_sp;
 
   const DWARFDebugMacroHeader &header =
       DWARFDebugMacroHeader::ParseHeader(debug_macro_data, offset);
   DWARFDebugMacroEntry::ReadMacroEntries(
-      debug_macro_data, m_context.getOrLoadStrData(), header.OffsetIs64Bit(),
-      offset, this, debug_macros_sp);
+      debug_macro_data, m_context.getOrLoadStrData(), str_offsets_info,
+      header.OffsetIs64Bit(), offset, this, debug_macros_sp);
 
   return debug_macros_sp;
 }
 
 bool SymbolFileDWARF::ParseDebugMacros(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
 
-  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
+  DWARFUnit *dwarf_cu = &GetDWARFCompileUnit(&comp_unit)->GetNonSkeletonUnit();
   if (dwarf_cu == nullptr)
     return false;
 
   const DWARFBaseDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
   if (!dwarf_cu_die)
     return false;
 
   lldb::offset_t sect_offset =
       dwarf_cu_die.GetAttributeValueAsUnsigned(DW_AT_macros, DW_INVALID_OFFSET);
   if (sect_offset == DW_INVALID_OFFSET)
     sect_offset = dwarf_cu_die.GetAttributeValueAsUnsigned(DW_AT_GNU_macros,
                                                            DW_INVALID_OFFSET);
   if (sect_offset == DW_INVALID_OFFSET)
     return false;
 
-  comp_unit.SetDebugMacros(ParseDebugMacros(&sect_offset));
+  std::unique_ptr<DWARFStrOffsetsInfo> str_offsets_info;
+  lldb::offset_t cu_str_offset = dwarf_cu->GetStrOffsetsBase();
+  SymbolFileDWARF &symfile = dwarf_cu->GetSymbolFileDWARF();
+  if (cu_str_offset && cu_str_offset != DW_INVALID_OFFSET)
+    str_offsets_info = std::make_unique<DWARFStrOffsetsInfo>(
+        cu_str_offset, symfile.GetDWARFContext().getOrLoadStrOffsetsData());
 
+  comp_unit.SetDebugMacros(
+      symfile.ParseDebugMacros(&sect_offset, str_offsets_info.get()));
   return true;
 }
 
 size_t SymbolFileDWARF::ParseBlocksRecursive(
     lldb_private::CompileUnit &comp_unit, Block *parent_block,
     const DWARFDIE &orig_die, addr_t subprogram_low_pc, uint32_t depth) {
   size_t blocks_added = 0;
   DWARFDIE die = orig_die;

lldb/test/API/commands/expression/macros/Makefile

 CXX_SOURCES := main.cpp
 
 DEBUG_INFO_FLAG = -g3 -gdwarf-5
 
 # GCC produces incorrect .debug_macro section when "-include" option is used:
 # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93075.
 NO_TEST_COMMON_H := 1
+CFLAGS_EXTRAS := -fdebug-macro
 
 include Makefile.rules

lldb/test/API/commands/expression/macros/TestMacros.py

 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class TestMacros(TestBase):
 
     @expectedFailureAll(
-        compiler="clang",
-        bugnumber="clang does not emit .debug_macro[.dwo] sections.")
-    @expectedFailureAll(
-        debug_info="dwo",
-        bugnumber="GCC produces multiple .debug_macro.dwo sections and the spec is unclear as to what it means")
-    @expectedFailureAll(
         hostoslist=["windows"],
         compiler="gcc",
         triple='.*-android')
     @expectedFailureAll(
         compiler="gcc", compiler_version=['<', '5.1'],
         bugnumber=".debug_macro was introduced in DWARF 5, GCC supports it since version 5.1")
     def test_expr_with_macros(self):
         self.build()

lldb/test/API/lang/c/macro/Makefile

+C_SOURCES := main.c
+CFLAGS_EXTRAS := -fdebug-macro
+
+include Makefile.rules

lldb/test/API/lang/c/macro/TestMacro.py

+"""Tests lldb macro evaluation."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MacroTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line number to break inside main().
+        self.line = line_number('main.c', '// Set break point at this line.')
+
+    def test(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Break inside the main.
+        lldbutil.run_break_set_by_file_and_line(
+            self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+        self.expect("expr DM + DF(10)", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['int', '62'])
+

lldb/test/API/lang/c/macro/main.c

+#define DM 10
+#define DF(x) (42 + (x))
+
+int main (int argc, char const *argv[])
+{
+    return 0; //// Set break point at this line.
+}