diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 1d49358b025e..b38886fe9bd9 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -410,6 +410,8 @@ class DeclSpec { /// otherwise, it is the same as TSTLoc. Hence, the pair TSTLoc and /// TSTNameLoc provides source range info for tag types. SourceLocation TSTNameLoc; + // Corresponds to PointerTypeLoc. + SourceLocation CheckedPtrKWLoc, CheckedPtrLeftSymLoc, CheckedPtrRightSymLoc; SourceRange TypeofParensRange; SourceLocation TQ_constLoc, TQ_restrictLoc, TQ_volatileLoc, TQ_atomicLoc, TQ_unalignedLoc; @@ -562,6 +564,13 @@ class DeclSpec { assert(isDeclRep((TST) TypeSpecType) || TypeSpecType == TST_typename); return TSTNameLoc; } + SourceLocation getCheckedPtrKWLoc() const { return CheckedPtrKWLoc; } + SourceLocation getCheckedPtrLeftSymLoc() const { + return CheckedPtrLeftSymLoc; + } + SourceLocation getCheckedPtrRightSymLoc() const { + return CheckedPtrRightSymLoc; + } SourceRange getTypeofParensRange() const { return TypeofParensRange; } void setTypeofParensRange(SourceRange range) { TypeofParensRange = range; } @@ -843,6 +852,9 @@ class DeclSpec { bool SetConstexprSpec(ConstexprSpecKind ConstexprKind, SourceLocation Loc, const char *&PrevSpec, unsigned &DiagID); + void setSpecCheckedPtr(SourceLocation KWLoc, SourceLocation LeftSymLoc, + SourceLocation RightSymLoc); + bool isFriendSpecified() const { return Friend_specified; } SourceLocation getFriendSpecLoc() const { return FriendLoc; } diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 2e699c354439..9227bf7e8116 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -2267,7 +2267,7 @@ FVComponentVariable::FVComponentVariable(const QualType &QT, const SourceManager &SM = C.getSourceManager(); SourceLocation DLoc = D->getLocation(); CharSourceRange CSR; - if (SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)) { + if (false /*SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)*/) { // It's not clear to me why, but the end of the SourceRange for the // declaration can come before the SourceLocation for the declaration. // This can result in SourceDeclaration failing to capture the whole diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index 780455412cd0..51e6cc811c07 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -622,6 +622,7 @@ SourceRange getDeclSourceRangeWithAnnotations( return SR; SourceLocation DeclEnd = SR.getEnd(); +#if 0 // Partial workaround for a compiler bug where if D has certain checked // pointer types such as `_Ptr` (seen in the partial_checked.c // regression test), D->getSourceRange() returns only the _Ptr token (TODO: @@ -631,6 +632,7 @@ SourceRange getDeclSourceRangeWithAnnotations( SourceLocation DeclLoc = D->getLocation(); if (SM.isBeforeInTranslationUnit(DeclEnd, DeclLoc)) DeclEnd = DeclLoc; +#endif SourceLocation AnnotationsEnd = getCheckedCAnnotationsEnd(D); if (AnnotationsEnd.isValid() && diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index a07076111819..d8b209ce355b 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1900,9 +1900,14 @@ static bool typeIsPostfix(QualType QT) { switch (T->getTypeClass()) { default: return false; - case Type::Pointer: - QT = cast(T)->getPointeeType(); + case Type::Pointer: { + const PointerType *PT = cast(T); + if (PT->isChecked()) + // See the comment about checked pointers in TypeLoc::getBeginLoc. + return false; + QT = PT->getPointeeType(); break; + } case Type::BlockPointer: QT = cast(T)->getPointeeType(); break; diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp index 222b1abac510..2fd803f48725 100644 --- a/clang/lib/AST/TypeLoc.cpp +++ b/clang/lib/AST/TypeLoc.cpp @@ -212,6 +212,19 @@ SourceLocation TypeLoc::getBeginLoc() const { case Qualified: Cur = Cur.getNextTypeLoc(); continue; + case Pointer: + if (Cur.castAs().getTypePtr()->isChecked()) { + // Unlike the usual "inside-out" C declarator syntax, checked pointer + // types (_Ptr, etc.) are "right-side-out". Since T is syntactically + // contained in _Ptr, it can't affect the source range of the whole + // type. Thus, the source range code can ignore T and treat _Ptr the + // same way as a single-token type such as a primitive or typedef. Here, + // a single-token type would hit the `default` case with + // Cur.getNextTypeLoc().isNull() returning true. + LeftMost = Cur; + break; + } + LLVM_FALLTHROUGH; default: if (Cur.getLocalSourceRange().getBegin().isValid()) LeftMost = Cur; @@ -249,6 +262,13 @@ SourceLocation TypeLoc::getEndLoc() const { Last = Cur; break; case Pointer: + if (Cur.castAs().getTypePtr()->isChecked()) { + // See the comment about checked pointers in TypeLoc::getBeginLoc. + if (!Last) + Last = Cur; + return Last.getLocalSourceRange().getEnd(); + } + LLVM_FALLTHROUGH; case BlockPointer: case MemberPointer: case LValueReference: diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 34d2d59ac845..d29653d59285 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -7837,6 +7837,7 @@ void Parser::ParseCheckedPointerSpecifiers(DeclSpec &DS) { "Not a checked pointer specifier"); tok::TokenKind Kind = Tok.getKind(); SourceLocation StartLoc = ConsumeToken(); + SourceLocation LeftLoc = Tok.getLocation(); if (ExpectAndConsume(tok::less)) { return; } @@ -7884,6 +7885,7 @@ void Parser::ParseCheckedPointerSpecifiers(DeclSpec &DS) { DiagID, Result.get(), Actions.getASTContext().getPrintingPolicy())) Diag(StartLoc, DiagID) << PrevSpec; + DS.setSpecCheckedPtr(StartLoc, LeftLoc, EndLoc); } /// [Checked C] Parse a specifier for an existential type. diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp index fb146aec724e..a31a145c3c3a 100644 --- a/clang/lib/Sema/DeclSpec.cpp +++ b/clang/lib/Sema/DeclSpec.cpp @@ -1184,6 +1184,14 @@ bool DeclSpec::SetConstexprSpec(ConstexprSpecKind ConstexprKind, return false; } +void DeclSpec::setSpecCheckedPtr(SourceLocation KWLoc, + SourceLocation LeftSymLoc, + SourceLocation RightSymLoc) { + this->CheckedPtrKWLoc = KWLoc; + this->CheckedPtrLeftSymLoc = LeftSymLoc; + this->CheckedPtrRightSymLoc = RightSymLoc; +} + void DeclSpec::SaveWrittenBuiltinSpecs() { writtenBS.Sign = static_cast(getTypeSpecSign()); writtenBS.Width = static_cast(getTypeSpecWidth()); diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 3d67743895f1..3f50fbdd9f20 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -6289,6 +6289,16 @@ namespace { TL.setNameLoc(DS.getTypeSpecTypeLoc()); } + void VisitPointerTypeLoc(PointerTypeLoc TL) { + // REVIEW: Should we have any error checks here? + TL.setKWLoc(DS.getCheckedPtrKWLoc()); + TL.setLeftSymLoc(DS.getCheckedPtrLeftSymLoc()); + TL.setRightSymLoc(DS.getCheckedPtrRightSymLoc()); + TypeSourceInfo *TInfo = nullptr; + Sema::GetTypeFromParser(DS.getRepAsType(), &TInfo); + TL.getNextTypeLoc().initializeFullCopy(TInfo->getTypeLoc()); + } + void VisitTypeLoc(TypeLoc TL) { // FIXME: add other typespec types and change this to an assert. TL.initialize(Context, DS.getTypeSpecTypeLoc()); diff --git a/clang/test/3C/partial_checked.c b/clang/test/3C/partial_checked.c index d288ac94ea06..06c9c5e9184c 100644 --- a/clang/test/3C/partial_checked.c +++ b/clang/test/3C/partial_checked.c @@ -76,6 +76,13 @@ void test4() { // getDeclSourceRangeWithAnnotations for more information. _Ptr gm; // CHECK: _Ptr<_Ptr (void)> gm = ((void *)0); +_Ptr gma[10]; +// CHECK_NOALL: _Ptr<_Ptr (void)> gma[10] = {((void *)0)}; +// CHECK_ALL: _Ptr<_Ptr (void)> gma _Checked[10] = {((void *)0)}; + +// TODO: Better names and CHECK comments +void gmf(_Ptr a, int *b) {} +void gmaf(_Ptr a[10], int *b) {} void test5(_Ptr a, _Ptr b, _Ptr<_Ptr> c, int **d) { // CHECK: void test5(_Ptr<_Ptr> a, _Ptr b : itype(_Ptr<_Ptr>), _Ptr<_Ptr> c, _Ptr<_Ptr> d) { diff --git a/clang/unittests/AST/SourceLocationTest.cpp b/clang/unittests/AST/SourceLocationTest.cpp index c1565c8f4c30..a477cb19e148 100644 --- a/clang/unittests/AST/SourceLocationTest.cpp +++ b/clang/unittests/AST/SourceLocationTest.cpp @@ -293,6 +293,80 @@ TEST(TypeLoc, LongLongConstRange) { EXPECT_TRUE(Verifier.match("long long const a = 0;", typeLoc())); } +// For multi-level types such as pointers, if we just used typeLoc() as above, +// it would match the TypeLocs of all the levels and the RangeVerifier would +// verify an arbitrary one of them. Instead, we want to verify a specific level. +internal::BindableMatcher typeLocAtDepth(unsigned int Depth) { + internal::BindableMatcher Matcher = typeLoc(hasParent(varDecl())); + for (unsigned int I = 0; I < Depth; I++) + Matcher = typeLoc(hasParent(Matcher)); + return Matcher; +} + +// Test the source range of a checked pointer type and all levels of its +// pointee. +TEST(CheckedPtr, TypeLoc) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 11); + EXPECT_TRUE(Verifier.match("_Ptr p;", typeLocAtDepth(0), Lang_C99)); +} +TEST(CheckedPtr, TypeLocPointee) { + RangeVerifier Verifier; + Verifier.expectRange(1, 6, 1, 10); + EXPECT_TRUE(Verifier.match("_Ptr p;", typeLocAtDepth(1), Lang_C99)); +} +TEST(CheckedPtr, TypeLocPointee2) { + RangeVerifier Verifier; + Verifier.expectRange(1, 6, 1, 6); + EXPECT_TRUE(Verifier.match("_Ptr p;", typeLocAtDepth(2), Lang_C99)); +} + +// Tests of checked pointers in combination with arrays (as an example of a type +// layer that uses the usual inside-out C declarator syntax). Also, some +// corresponding tests of unchecked pointers to verify that the special handling +// of checked pointers doesn't unintentionally affect unchecked pointers. +// +// For each example declaration, we test the source ranges of both the TypeLoc +// and the VarDecl. The VarDecl uses DeclaratorDecl::getSourceRange, which uses +// typeIsPostfix to decide whether the type extends past the name in order to +// choose either the name or the end of the type as the end of the source range. +// typeIsPostfix has logic roughly parallel to TypeLoc::getEndLoc. (Could the +// code be refactored to remove this duplication?) + +TEST(CheckedPtr, ArrayTypeLoc) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 13); + EXPECT_TRUE(Verifier.match("_Ptr p;", typeLocAtDepth(0), Lang_C99)); +} +TEST(CheckedPtr, ArrayDecl) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 15); + EXPECT_TRUE(Verifier.match("_Ptr p;", varDecl(), Lang_C99)); +} +TEST(CheckedPtr, UncheckedArrayTypeLoc) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 12); + EXPECT_TRUE(Verifier.match("int (*p)[10];", typeLocAtDepth(0), Lang_C99)); +} +TEST(CheckedPtr, UncheckedArrayDecl) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 12); + EXPECT_TRUE(Verifier.match("int (*p)[10];", varDecl(), Lang_C99)); +} + +// Test with array levels both inside and outside a checked pointer level. +TEST(CheckedPtr, SandwichArrayTypeLoc) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 18); + EXPECT_TRUE( + Verifier.match("_Ptr p[1];", typeLocAtDepth(0), Lang_C99)); +} +TEST(CheckedPtr, SandwichArrayDecl) { + RangeVerifier Verifier; + Verifier.expectRange(1, 1, 1, 18); + EXPECT_TRUE(Verifier.match("_Ptr p[1];", varDecl(), Lang_C99)); +} + TEST(CXXConstructorDecl, NoRetFunTypeLocRange) { RangeVerifier Verifier; Verifier.expectRange(1, 11, 1, 13);