[lldb] Fix some C++ virtual method call bugs in LLDB expression evaluation by

building method override tables for CXXMethodDecls in 
DWARFASTParserClang::CompleteTypeFromDWARF.

C++ virtual method calls in LLDB expressions may fail if the override table for
the method being called is not correct as IRGen will produce references to the
wrong (or a missing) vtable entry.

This patch does not fix calls to virtual methods with covariant return types as
it mistakenly treats these as overloads, rather than overrides. This will be
addressed in a future patch.

Review: https://reviews.llvm.org/D41997

Partially fixes <rdar://problem/14205774>

llvm-svn: 323163

lldb/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile

@@ -0,0 +1,8 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
+
+clean::
+	rm -rf $(wildcard *.o *.d *.dSYM)

lldb/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py

@@ -0,0 +1,47 @@
+"""
+Test calling an overriden method.
+
+Note:
+  This verifies that LLDB is correctly building the method overrides table.
+  If this table is not built correctly then calls to overridden methods in
+  derived classes may generate references to non-existant vtable entries,
+  as the compiler treats the overridden method as a totally new virtual
+  method definition.
+  <rdar://problem/14205774>
+
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ExprCommandCallOverriddenMethod(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line number to break for main.c.
+        self.line = line_number('main.cpp', '// Set breakpoint here')
+
+    def test(self):
+        """Test calls to overridden methods in derived classes."""
+        self.build()
+
+        # Set breakpoint in main and run exe
+        self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "main.cpp", self.line, num_expected_locations=-1, loc_exact=True)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # Test call to method in base class (this should always work as the base
+        # class method is never an override).
+        self.expect("expr b->foo()")
+
+        # Test call to overridden method in derived class (this will fail if the
+        # overrides table is not correctly set up, as Derived::foo will be assigned
+        # a vtable entry that does not exist in the compiled program).
+        self.expect("expr d.foo()")

lldb/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp

@@ -0,0 +1,16 @@
+class Base {
+public:
+  virtual ~Base() {}
+  virtual void foo() {}
+};
+
+class Derived : public Base {
+public:
+  virtual void foo() {}
+};
+
+int main() {
+  Derived d;
+  Base *b = &d;
+  return 0; // Set breakpoint here
+}

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

@@ -40,6 +40,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 
@@ -2103,6 +2104,92 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 
+// Checks whether m1 is an overload of m2 (as opposed to an override).
+// This is called by addOverridesForMethod to distinguish overrides (which share
+// a vtable entry) from overloads (which require distinct entries).
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  lldbassert(&m1->getASTContext() == &m2->getASTContext() &&
+             "Methods should have the same AST context");
+  clang::ASTContext &context = m1->getASTContext();
+
+  const auto *m1Type =
+    llvm::cast<clang::FunctionProtoType>(
+      context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type =
+    llvm::cast<clang::FunctionProtoType>(
+      context.getCanonicalType(m2->getType()));
+
+  auto compareArgTypes =
+    [&context](const clang::QualType &m1p, const clang::QualType &m2p) {
+      return context.hasSameType(m1p.getUnqualifiedType(),
+                                 m2p.getUnqualifiedType());
+    };
+
+  return !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(),
+                     m2Type->param_type_begin(), compareArgTypes);
+}
+
+// If decl is a virtual method, walk the base classes looking for methods that
+// decl overrides. This table of overridden methods is used by IRGen to determine
+// the vtable layout for decl's parent class.
+static void addOverridesForMethod(clang::CXXMethodDecl *decl) {
+  if (!decl->isVirtual())
+    return;
+
+  clang::CXXBasePaths paths;
+
+  auto find_overridden_methods =
+    [decl](const clang::CXXBaseSpecifier *specifier, clang::CXXBasePath &path) {
+      if (auto *base_record =
+          llvm::dyn_cast<clang::CXXRecordDecl>(
+            specifier->getType()->getAs<clang::RecordType>()->getDecl())) {
+
+        clang::DeclarationName name = decl->getDeclName();
+
+        // If this is a destructor, check whether the base class destructor is
+        // virtual.
+        if (name.getNameKind() == clang::DeclarationName::CXXDestructorName)
+          if (auto *baseDtorDecl = base_record->getDestructor()) {
+            if (baseDtorDecl->isVirtual()) {
+              path.Decls = baseDtorDecl;
+              return true;
+            } else
+              return false;
+          }
+
+        // Otherwise, search for name in the base class.
+        for (path.Decls = base_record->lookup(name); !path.Decls.empty();
+             path.Decls = path.Decls.slice(1)) {
+          if (auto *method_decl =
+                llvm::dyn_cast<clang::CXXMethodDecl>(path.Decls.front()))
+            if (method_decl->isVirtual() && !isOverload(decl, method_decl)) {
+              path.Decls = method_decl;
+              return true;
+            }
+        }
+      }
+
+      return false;
+    };
+
+  if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) {
+    for (auto *overridden_decl : paths.found_decls())
+      decl->addOverriddenMethod(
+        llvm::cast<clang::CXXMethodDecl>(overridden_decl));
+  }
+}
+
+// If clang_type is a CXXRecordDecl, builds the method override list for each
+// of its virtual methods.
+static void addMethodOverrides(ClangASTContext &ast, CompilerType &clang_type) {
+  if (auto *record =
+      ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType()))
+    for (auto *method : record->methods())
+      addOverridesForMethod(method);
+}
+
 bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
                                                 lldb_private::Type *type,
                                                 CompilerType &clang_type) {
@@ -2315,6 +2402,7 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die,
       }
     }
 
+    addMethodOverrides(m_ast, clang_type);
     ClangASTContext::BuildIndirectFields(clang_type);
     ClangASTContext::CompleteTagDeclarationDefinition(clang_type);