[Lldb-commits] [lldb] r323163 - [lldb] Fix some C++ virtual method call bugs in LLDB expression evaluation by
Lang Hames via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 22 15:53:56 PST 2018
Author: lhames
Date: Mon Jan 22 15:53:56 2018
New Revision: 323163
URL: http://llvm.org/viewvc/llvm-project?rev=323163&view=rev
Log:
[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>
Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/
lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile
lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py
lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Added: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile?rev=323163&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/Makefile Mon Jan 22 15:53:56 2018
@@ -0,0 +1,8 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
+
+clean::
+ rm -rf $(wildcard *.o *.d *.dSYM)
Added: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py?rev=323163&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py Mon Jan 22 15:53:56 2018
@@ -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()")
Added: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp?rev=323163&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp Mon Jan 22 15:53:56 2018
@@ -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
+}
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=323163&r1=323162&r2=323163&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Mon Jan 22 15:53:56 2018
@@ -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::ParseTemplateP
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::CompleteTypeFr
}
}
+ addMethodOverrides(m_ast, clang_type);
ClangASTContext::BuildIndirectFields(clang_type);
ClangASTContext::CompleteTagDeclarationDefinition(clang_type);
More information about the lldb-commits
mailing list