labath created this revision.
labath added reviewers: clayborg, JDevlieghere.
Herald added a subscriber: aprantl.

When searching for methods only, we need to do extra work to make sure
the functions we get from the apple tables are indeed methods.
Previously we were resolving the DIE into a SymbolContext and then
checked whether the enclosing CompilerDeclContext is a
class (or struct, or union).

This patch changes that to operate on the debug info directly. This
should be:

- simpler
- faster
- more consistent with the ManualDWARFIndex (which does the same check, only at 
indexing time).

What we lose this ways is for the language plugin to have a say in what
it considers to be a "class", but that's probably more flexibility than
we need (and if we really wanted to do that in the future, we could
implement a more direct way to consult the plugin about this).

This also fixes the find-method-local-struct test, which was failing
because we were not able to construct a CompilerDeclContext for a local
struct correctly.

As a drive-by, I rename the DWARFDIE's IsStructClassOrUnion method to
match the name on the CompilerDeclContext class.


https://reviews.llvm.org/D47470

Files:
  lit/SymbolFile/DWARF/find-method-local-struct.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -292,14 +292,14 @@
           bool is_method = false;
           if (parent) {
             DWARFDIE parent_die(&unit, parent);
-            if (parent_die.IsStructClassOrUnion())
+            if (parent_die.IsStructUnionOrClass())
               is_method = true;
             else {
               if (specification_die_form.IsValid()) {
                 DWARFDIE specification_die =
                     unit.GetSymbolFileDWARF()->DebugInfo()->GetDIE(
                         DIERef(specification_die_form));
-                if (specification_die.GetParent().IsStructClassOrUnion())
+                if (specification_die.GetParent().IsStructUnionOrClass())
                   is_method = true;
               }
             }
Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -19,7 +19,9 @@
   //----------------------------------------------------------------------
   // Tests
   //----------------------------------------------------------------------
-  bool IsStructClassOrUnion() const;
+  bool IsStructUnionOrClass() const;
+
+  bool IsMethod() const;
 
   //----------------------------------------------------------------------
   // Accessors
Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -209,12 +209,21 @@
     return DWARFDIE();
 }
 
-bool DWARFDIE::IsStructClassOrUnion() const {
+bool DWARFDIE::IsStructUnionOrClass() const {
   const dw_tag_t tag = Tag();
   return tag == DW_TAG_class_type || tag == DW_TAG_structure_type ||
          tag == DW_TAG_union_type;
 }
 
+bool DWARFDIE::IsMethod() const {
+  DWARFDIE parent = GetParent();
+  if (parent.IsStructUnionOrClass())
+    return true;
+  return GetReferencedDIE(DW_AT_specification)
+      .GetParent()
+      .IsStructUnionOrClass();
+}
+
 DWARFDIE
 DWARFDIE::GetContainingDWOModuleDIE() const {
   if (IsValid()) {
Index: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -146,6 +146,14 @@
     m_apple_namespaces_up->FindByName(name.GetStringRef(), offsets);
 }
 
+static bool KeepFunctionDIE(DWARFDIE die, uint32_t name_type_mask) {
+  if (name_type_mask & eFunctionNameTypeMethod &&
+      name_type_mask & eFunctionNameTypeBase)
+    return true;
+  bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod;
+  return looking_for_methods == die.IsMethod();
+}
+
 void AppleDWARFIndex::GetFunctions(
     ConstString name, DWARFDebugInfo &info,
     llvm::function_ref<bool(const DWARFDIE &die, bool include_inlines,
@@ -228,47 +236,15 @@
         if (!SymbolFileDWARF::DIEInDeclContext(parent_decl_ctx, die))
           continue; // The containing decl contexts don't match
 
+        if (!KeepFunctionDIE(die, name_type_mask))
+          continue;
+
+        if (resolved_dies.find(die.GetDIE()) != resolved_dies.end())
+          continue;
+
         // If we get to here, the die is good, and we should add it:
-        if (resolved_dies.find(die.GetDIE()) == resolved_dies.end() &&
-            resolve_function(die, include_inlines, sc_list)) {
-          bool keep_die = true;
-          if ((name_type_mask &
-               (eFunctionNameTypeBase | eFunctionNameTypeMethod)) !=
-              (eFunctionNameTypeBase | eFunctionNameTypeMethod)) {
-            // We are looking for either basenames or methods, so we need
-            // to trim out the ones we won't want by looking at the type
-            SymbolContext sc;
-            if (sc_list.GetLastContext(sc)) {
-              if (sc.block) {
-                // We have an inlined function
-              } else if (sc.function) {
-                Type *type = sc.function->GetType();
-
-                if (type) {
-                  CompilerDeclContext decl_ctx =
-                      get_decl_context_containing_uid(type->GetID());
-                  if (decl_ctx.IsStructUnionOrClass()) {
-                    if (name_type_mask & eFunctionNameTypeBase) {
-                      sc_list.RemoveContextAtIndex(sc_list.GetSize() - 1);
-                      keep_die = false;
-                    }
-                  } else {
-                    if (name_type_mask & eFunctionNameTypeMethod) {
-                      sc_list.RemoveContextAtIndex(sc_list.GetSize() - 1);
-                      keep_die = false;
-                    }
-                  }
-                } else {
-                  m_module.ReportWarning(
-                      "function at die offset 0x%8.8x had no function type",
-                      die_ref.die_offset);
-                }
-              }
-            }
-          }
-          if (keep_die)
-            resolved_dies.insert(die.GetDIE());
-        }
+        if (resolve_function(die, include_inlines, sc_list))
+          resolved_dies.insert(die.GetDIE());
       } else
         ReportInvalidDIEOffset(die_ref.die_offset, name.GetStringRef());
     }
Index: lit/SymbolFile/DWARF/find-method-local-struct.cpp
===================================================================
--- lit/SymbolFile/DWARF/find-method-local-struct.cpp
+++ lit/SymbolFile/DWARF/find-method-local-struct.cpp
@@ -1,6 +1,3 @@
-// llvm.org/pr37537
-// XFAIL: *
-
 // RUN: clang %s -g -c -o %t --target=x86_64-apple-macosx
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \
 // RUN:   FileCheck %s
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to