Skip to content

Commit

Permalink
[clang][ASTImporter] Fix import of typedef with unnamed structures
Browse files Browse the repository at this point in the history
Fix crash in ASTImporter related to import of unnamed structures and typedefs
to these maybe with pointer.
There was a series of problems exposed by https://reviews.llvm.org/D133468
(commit 69a6417) in the ASTImporter breaking
cross-translation unit analysis. This change fixes one of the problems exposed
by that change for importing unnamed structures. The problem was
discovered when running clang static analysis on open source projects
using cross-translation unit analysis.

Simple test command. Produces crash without change, passes all tests
with change.

```
ninja ASTTests && ./tools/clang/unittests/AST/ASTTests
  --gtest_filter="*/*ImportAnonymousStruct/0"
```

Formatted crash stack:

```
ASTTests: <root>/clang/lib/AST/ASTContext.cpp:4787:
  clang::QualType clang::ASTContext::getTypedefType(const clang::TypedefNameDecl*,
  clang::QualType) const: Assertion `hasSameType(Decl->getUnderlyingType(), Underlying)' failed.
...
 #9 <addr> clang::ASTContext::getTypedefType(clang::TypedefNameDecl const*, clang::QualType) const
             <root>/clang/lib/AST/ASTContext.cpp:4789:26
             <root>/clang/lib/AST/ASTImporter.cpp:1374:71
             <root>/tools/clang/include/clang/AST/TypeNodes.inc:75:1
             <root>/clang/lib/AST/ASTImporter.cpp:8663:8
```

Reviewed By: donat.nagy

Differential Revision: https://reviews.llvm.org/D145868
  • Loading branch information
balazske committed Apr 13, 2023
1 parent c1f7636 commit 9d0b55f
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
8 changes: 6 additions & 2 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1365,12 +1365,16 @@ ExpectedType ASTNodeImporter::VisitTypedefType(const TypedefType *T) {
Expected<TypedefNameDecl *> ToDeclOrErr = import(T->getDecl());
if (!ToDeclOrErr)
return ToDeclOrErr.takeError();

TypedefNameDecl *ToDecl = *ToDeclOrErr;
if (ToDecl->getTypeForDecl())
return QualType(ToDecl->getTypeForDecl(), 0);

ExpectedType ToUnderlyingTypeOrErr = import(T->desugar());
if (!ToUnderlyingTypeOrErr)
return ToUnderlyingTypeOrErr.takeError();

return Importer.getToContext().getTypedefType(*ToDeclOrErr,
*ToUnderlyingTypeOrErr);
return Importer.getToContext().getTypedefType(ToDecl, *ToUnderlyingTypeOrErr);
}

ExpectedType ASTNodeImporter::VisitTypeOfExprType(const TypeOfExprType *T) {
Expand Down
63 changes: 63 additions & 0 deletions clang/unittests/AST/ASTImporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8576,6 +8576,69 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportTwoTypedefsToUnnamedRecord) {
Typedef2->getUnderlyingType().getTypePtr());
}

TEST_P(ASTImporterOptionSpecificTestBase,
ImportExistingTypedefToUnnamedRecordPtr) {
const char *Code =
R"(
typedef const struct { int fff; } * const T;
extern T x;
)";
Decl *ToTU = getToTuDecl(Code, Lang_C99);
Decl *FromTU = getTuDecl(Code, Lang_C99);

auto *FromX =
FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("x")));
auto *ToX = Import(FromX, Lang_C99);
EXPECT_TRUE(ToX);

auto *Typedef1 =
FirstDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("T")));
auto *Typedef2 =
LastDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("T")));
// FIXME: These should be imported separately, like in the test above.
// Or: In the test above these should be merged too.
EXPECT_EQ(Typedef1, Typedef2);

auto *FromR = FirstDeclMatcher<RecordDecl>().match(
FromTU, recordDecl(hasDescendant(fieldDecl(hasName("fff")))));
auto *ToRExisting = FirstDeclMatcher<RecordDecl>().match(
ToTU, recordDecl(hasDescendant(fieldDecl(hasName("fff")))));
ASSERT_TRUE(FromR);
auto *ToRImported = Import(FromR, Lang_C99);
// FIXME: If typedefs are not imported separately, do not import ToRImported
// separately.
EXPECT_NE(ToRExisting, ToRImported);
}

TEST_P(ASTImporterOptionSpecificTestBase,
ImportTypedefWithDifferentUnderlyingType) {
const char *Code =
R"(
using X1 = int;
using Y1 = int;
using RPB1 = X1*;
typedef RPB1 RPX1;
using RPB1 = Y1*; // redeclared
typedef RPB1 RPY1;
auto X = 0 ? (RPX1){} : (RPY1){};
)";
Decl *ToTU = getToTuDecl("", Lang_CXX11);
Decl *FromTU = getTuDecl(Code, Lang_CXX11);

auto *FromX =
FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("X")));

auto *FromXType = FromX->getType()->getAs<TypedefType>();
EXPECT_FALSE(FromXType->typeMatchesDecl());

auto *ToX = Import(FromX, Lang_CXX11);
auto *ToXType = ToX->getType()->getAs<TypedefType>();
// FIXME: This should be false.
EXPECT_TRUE(ToXType->typeMatchesDecl());
}

INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);

Expand Down

0 comments on commit 9d0b55f

Please sign in to comment.