lhames updated this revision to Diff 130284. lhames added a comment. Updated to address review comments:
- assert changed to lldbassert - comments added - test case breakpoint comment simplified - unused import removed from testcase - testcase Makefile cleaned up Repository: rL LLVM https://reviews.llvm.org/D41997 Files: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Python/lldbsuite/test/expression_command/call-overridden-method/Makefile Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp
Index: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ 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" @@ -2099,6 +2100,92 @@ 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) { @@ -2311,6 +2398,7 @@ } } + addMethodOverrides(m_ast, clang_type); ClangASTContext::BuildIndirectFields(clang_type); ClangASTContext::CompleteTagDeclarationDefinition(clang_type); Index: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp =================================================================== --- /dev/null +++ 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 +} Index: Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py =================================================================== --- /dev/null +++ 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()") Index: Python/lldbsuite/test/expression_command/call-overridden-method/Makefile =================================================================== --- /dev/null +++ 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-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits