Author: shafik Date: 2020-09-21T14:57:00-07:00 New Revision: 6807f244fa67bb75ef09fb3db54743b5b358a7fa
URL: https://github.com/llvm/llvm-project/commit/6807f244fa67bb75ef09fb3db54743b5b358a7fa DIFF: https://github.com/llvm/llvm-project/commit/6807f244fa67bb75ef09fb3db54743b5b358a7fa.diff LOG: [ASTImporter] Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl When we fixed ImportDeclContext(...) in D71378 to make sure we complete each FieldDecl of a RecordDecl when we are importing the definition we missed the case where a FeildDecl was an ArrayType whose ElementType is a record. This fix was motivated by a codegen crash during LLDB expression parsing. Since we were not importing the definition we were crashing during layout which required all the records be defined. Differential Revision: https://reviews.llvm.org/D86660 Added: lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/Makefile lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/TestImportDefinitionArrayType.py lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp Modified: clang/lib/AST/ASTImporter.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 11bf629ecdde2..60b6a9706d6ac 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1742,12 +1742,28 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) { Decl *ImportedDecl = *ImportedOrErr; FieldDecl *FieldTo = dyn_cast_or_null<FieldDecl>(ImportedDecl); if (FieldFrom && FieldTo) { - const RecordType *RecordFrom = FieldFrom->getType()->getAs<RecordType>(); - const RecordType *RecordTo = FieldTo->getType()->getAs<RecordType>(); - if (RecordFrom && RecordTo) { - RecordDecl *FromRecordDecl = RecordFrom->getDecl(); - RecordDecl *ToRecordDecl = RecordTo->getDecl(); + RecordDecl *FromRecordDecl = nullptr; + RecordDecl *ToRecordDecl = nullptr; + // If we have a field that is an ArrayType we need to check if the array + // element is a RecordDecl and if so we need to import the defintion. + if (FieldFrom->getType()->isArrayType()) { + // getBaseElementTypeUnsafe(...) handles multi-dimensonal arrays for us. + FromRecordDecl = FieldFrom->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl(); + ToRecordDecl = FieldTo->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl(); + } + + if (!FromRecordDecl || !ToRecordDecl) { + const RecordType *RecordFrom = + FieldFrom->getType()->getAs<RecordType>(); + const RecordType *RecordTo = FieldTo->getType()->getAs<RecordType>(); + + if (RecordFrom && RecordTo) { + FromRecordDecl = RecordFrom->getDecl(); + ToRecordDecl = RecordTo->getDecl(); + } + } + if (FromRecordDecl && ToRecordDecl) { if (FromRecordDecl->isCompleteDefinition() && !ToRecordDecl->isCompleteDefinition()) { Error Err = ImportDefinition(FromRecordDecl, ToRecordDecl); diff --git a/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/Makefile b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/TestImportDefinitionArrayType.py b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/TestImportDefinitionArrayType.py new file mode 100644 index 0000000000000..a485b94a680dc --- /dev/null +++ b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/TestImportDefinitionArrayType.py @@ -0,0 +1,14 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestImportDefinitionArrayType(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp")) + + self.expect_expr("__private->o", result_type="char", result_value="'A'") diff --git a/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp new file mode 100644 index 0000000000000..99fc7b727cc80 --- /dev/null +++ b/lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp @@ -0,0 +1,52 @@ +// This is a reproducer for a crash during codegen. The base issue is when we +// Import the DeclContext we force FieldDecl that are RecordType to be defined +// since we need these to be defined in order to layout the class. +// This case involves an array member whose ElementType are records. In this +// case we need to check the ElementType of an ArrayType and if it is a record +// we need to import the definition. +struct A { + int x; +}; + +struct B { + // When we import the all the FieldDecl we need to check if we have an + // ArrayType and then check if the ElementType is a RecordDecl and if so + // import the defintion. Otherwise during codegen we will attempt to layout A + // but won't be able to. + A s1[2]; + A s2[2][2][3]; + char o; +}; + +class FB { +public: + union { + struct { + unsigned char *_s; + } t; + char *tt[1]; + } U; + + FB(B *p) : __private(p) {} + + // We import A but we don't import the definition. + void f(A **bounds) {} + + void init(); + +private: + B *__private; +}; + +void FB::init() { + return; // break here +} + +int main() { + B b; + FB fb(&b); + + b.o = 'A'; + + fb.init(); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
