[SymbolFile] Implement GetCompleteObjCClass for .debug_names

[JDevlieghere]

Summary

When running the test suite with .debug_names a bunch of tests were failing. Part of that is solved by implementing the missing GetCompleteObjCClass in DebugNamesDWARFIndex.

[aprantl]

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

157 ↗

(On Diff #152898)

Wait.. we accept both structs and classes in LLDB? Should we also switch over to using DW_TAG_class in clang since that sounds more appropriate anyway?

[clayborg]

The function is looking for the complete objective C type. The code needs to be modified to return the complete type only if one is found, else just one of the other incomplete versions is needed, not many.

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

166 ↗

(On Diff #152898)

Why are we pushing a potentially incomplete ObjC type here? We should keep a list of incomplete types and if we don't find a complete one, then return that as a fallback.

173 ↗

(On Diff #152898)

Why are pushing a DIERef that doesn't produce a DIE? Remove the push_back.

178 ↗

(On Diff #152898)

If we find this, return right away with only one result: the complete version.

180 ↗

(On Diff #152898)

We need to add code here to return an incomplete version of the ObjC type from line 166 if no complete versions are found.

[JDevlieghere]

In D48596#1143666, @clayborg wrote:

The function is looking for the complete objective C type. The code needs to be modified to return the complete type only if one is found, else just one of the other incomplete versions is needed, not many.

Thanks Greg, looks like I misunderstood the code. I'll update the patch.

[JDevlieghere]

• Feedback Greg

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

157 ↗

(On Diff #152898)

I'm probably not the right person to answer this. I copied this from HashedNameToDIE:

611       // We don't only want the one true definition, so try and see what we can                                                                                                                                                                                                                                                                  612       // find, and only return class or struct DIEs.
...
616       DWARFMappedHash::ExtractClassOrStructDIEArray(

[labath]

How are you planning on testing this? Most of the DWARFIndex functionality is tested in terms of lldb-test, but I think this function is not reachable from there. Maybe you could write a specific dotest-test which targets this?

[JDevlieghere]

• Add test case for debug_names variant.

[labath]

looks good to me

[clayborg]

Looks good.

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/forward-decl/TestForwardDecl.py

class ForwardDeclTestCase(TestBase):
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.source = 'main.m'
         self.line = line_number(self.source, '// Set breakpoint 0 here.')
         self.shlib_names = ["Container"]
 
-    @skipUnlessDarwin
-    def test_expr(self):
-        self.build()
+    def do_test(self, dictionary=None):
+        self.build(dictionary=dictionary)
 
         # Create a target by the debugger.
         target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
         self.assertTrue(target, VALID_TARGET)
 
         # Create the breakpoint inside function 'main'.
         breakpoint = target.BreakpointCreateByLocation(self.source, self.line)
         self.assertTrue(breakpoint, VALID_BREAKPOINT)
def do_test(self, dictionary=None):
 
         # The breakpoint should have a hit count of 1.
         self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
                     substrs=[' resolved, hit count = 1'])
 
         # This should display correctly.
         self.expect("expression [j getMember]", VARIABLES_DISPLAYED_CORRECTLY,
                     substrs=["= 0x"])
+
+    @skipUnlessDarwin
+    def test_expr(self):
+        self.do_test()
+
+    @no_debug_info_test
+    @skipUnlessDarwin
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "7.0"])
+    def test_debug_names(self):
+        """Test that we are able to find complete types when using DWARF v5
+        accelerator tables"""
+        self.do_test(
+            dict(CFLAGS_EXTRAS="-dwarf-version=5 -mllvm -accel-tables=Dwarf"))

lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

public:
   void Preload() override { m_fallback.Preload(); }
 
   void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression &regex,
                           DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override {}
   void GetCompleteObjCClass(ConstString class_name, bool must_be_implementation,
-                            DIEArray &offsets) override {}
+                            DIEArray &offsets) override;
   void GetTypes(ConstString name, DIEArray &offsets) override;
   void GetTypes(const DWARFDeclContext &context, DIEArray &offsets) override;
   void GetNamespaces(ConstString name, DIEArray &offsets) override;
   void GetFunctions(ConstString name, DWARFDebugInfo &info,
                     const CompilerDeclContext &parent_decl_ctx,
                     uint32_t name_type_mask,
                     std::vector<DWARFDIE> &dies) override;
   void GetFunctions(const RegularExpression &regex,

lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

for (DebugNames::NameTableEntry nte: ni) {
 
         Append(*entry_or, offsets);
       }
       MaybeLogLookupError(entry_or.takeError(), ni, nte.getString());
     }
   }
 }
 
+void DebugNamesDWARFIndex::GetCompleteObjCClass(ConstString class_name,
+                                                bool must_be_implementation,
+                                                DIEArray &offsets) {
+  m_fallback.GetCompleteObjCClass(class_name, must_be_implementation, offsets);
+
+  // Keep a list of incomplete types as fallback for when we don't find the
+  // complete type.
+  DIEArray incomplete_types;
+
+  for (const DebugNames::Entry &entry :
+       m_debug_names_up->equal_range(class_name.GetStringRef())) {
+    if (entry.tag() != DW_TAG_structure_type &&
+        entry.tag() != DW_TAG_class_type)
+      continue;
+
+    DIERef ref = ToDIERef(entry);
+    if (!ref)
+      continue;
+
+    DWARFUnit *cu = m_debug_info.GetCompileUnit(ref.cu_offset);
+    if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
+      incomplete_types.push_back(ref);
+      continue;
+    }
+
+    // FIXME: We should return DWARFDIEs so we don't have to resolve it twice.
+    DWARFDIE die = m_debug_info.GetDIE(ref);
+    if (!die)
+      continue;
+
+    if (die.GetAttributeValueAsUnsigned(DW_AT_APPLE_objc_complete_type, 0)) {
+      // If we find the complete version we're done.
+      offsets.push_back(ref);
+      return;
+    } else {
+      incomplete_types.push_back(ref);
+    }
+  }
+
+  offsets.insert(offsets.end(), incomplete_types.begin(),
+                 incomplete_types.end());
+}
+
 void DebugNamesDWARFIndex::GetTypes(ConstString name, DIEArray &offsets) {
   m_fallback.GetTypes(name, offsets);
 
   for (const DebugNames::Entry &entry :
        m_debug_names_up->equal_range(name.GetStringRef())) {
     if (isType(entry.tag()))
       Append(entry, offsets);
   }