[clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

[vsapsai]

Summary

Some TypeLocs are considered "sugar" and we go past them in
GetTypeSourceInfoForDeclarator. The problem is that we peel off only
the same kind of TypeLoc at the time which makes it impossible to
handle mixed sequences like
AttributedTypeLoc - MacroQualifiedTypeLoc - AttributedTypeLoc - PointerTypeLoc

In this situation, as shown in the added test, we don't get to
PointerTypeLoc and don't set its starLoc leaving it uninitialized.

Address FIXME and peel off "sugar" TypeLocs regardless of their order.

rdar://102149264

[vsapsai]

clang-format the change.

[vsapsai]

Want to note that originally the issue was noticed with the modules because we serialize and deserialize PointerTypeLoc::starLoc and on deserialization we assert if the value is too big. With uninitialized memory the value can be too big sometimes and clang was failing intermittently. For testing don't use modules anymore and call PointerTypeLoc::getStarLoc directly to make it simpler and more reliable.

clang/lib/Sema/SemaType.cpp

6513–6516

Didn't move AtomicTypeLoc into the switch because it is peeled off only once and not in the while loop unlike other TypeLocs.

[vsapsai]

Rebase the patch and trigger pre-commit checks.

[yonghong-song]

LGTM. Thanks!

[vsapsai]

Thanks for the review!

clang/lib/Sema/SemaType.cpp

GetTypeSourceInfoForDeclarator(TypeProcessingState &State,
   if (isa<PackExpansionType>(T)) {
     CurrTL.castAs<PackExpansionTypeLoc>().setEllipsisLoc(D.getEllipsisLoc());
     CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
   }
 
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
     // An AtomicTypeLoc might be produced by an atomic qualifier in this
     // declarator chunk.
     if (AtomicTypeLoc ATL = CurrTL.getAs<AtomicTypeLoc>()) {
       fillAtomicQualLoc(ATL, D.getTypeObject(i));
       CurrTL = ATL.getValueLoc().getUnqualifiedLoc();
     }
 
-    while (MacroQualifiedTypeLoc TL = CurrTL.getAs<MacroQualifiedTypeLoc>()) {
+    bool HasDesugaredTypeLoc = true;
+    while (HasDesugaredTypeLoc) {
+      switch (CurrTL.getTypeLocClass()) {
+      case TypeLoc::MacroQualified: {
+        auto TL = CurrTL.castAs<MacroQualifiedTypeLoc>();
         TL.setExpansionLoc(
             State.getExpansionLocForMacroQualifiedType(TL.getTypePtr()));
         CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+        break;
       }
 
-    while (AttributedTypeLoc TL = CurrTL.getAs<AttributedTypeLoc>()) {
+      case TypeLoc::Attributed: {
+        auto TL = CurrTL.castAs<AttributedTypeLoc>();
         fillAttributedTypeLoc(TL, State);
         CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+        break;
       }
 
-    while (BTFTagAttributedTypeLoc TL = CurrTL.getAs<BTFTagAttributedTypeLoc>())
-      CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+      case TypeLoc::Adjusted:
+      case TypeLoc::BTFTagAttributed: {
+        CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
+        break;
+      }
 
-    while (DependentAddressSpaceTypeLoc TL =
-               CurrTL.getAs<DependentAddressSpaceTypeLoc>()) {
+      case TypeLoc::DependentAddressSpace: {
+        auto TL = CurrTL.castAs<DependentAddressSpaceTypeLoc>();
         fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
         CurrTL = TL.getPointeeTypeLoc().getUnqualifiedLoc();
+        break;
       }
 
-    // FIXME: Ordering here?
-    while (AdjustedTypeLoc TL = CurrTL.getAs<AdjustedTypeLoc>())
-      CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+      default:
+        HasDesugaredTypeLoc = false;
+        break;
+      }
+    }
 
     DeclaratorLocFiller(S.Context, State, D.getTypeObject(i)).Visit(CurrTL);
     CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
   }
 
   // If we have different source information for the return type, use
   // that.  This really only applies to C++ conversion functions.
   if (ReturnTypeInfo) {

clang/unittests/AST/SourceLocationTest.cpp

TEST(UnaryTransformTypeLoc, ParensRange) {
   EXPECT_TRUE(Verifier.match(
       "template <typename T>\n"
       "struct S {\n"
       "typedef __underlying_type(T) type;\n"
       "};",
       loc(unaryTransformType())));
 }
 
+TEST(PointerTypeLoc, StarLoc) {
+  llvm::Annotations Example(R"c(
+    int $star^*var;
+  )c");
+
+  auto AST = tooling::buildASTFromCode(Example.code());
+  SourceManager &SM = AST->getSourceManager();
+  auto &Ctx = AST->getASTContext();
+
+  auto *VD = selectFirst<VarDecl>("vd", match(varDecl(hasName("var")).bind("vd"), Ctx));
+  ASSERT_NE(VD, nullptr);
+
+  auto TL =
+      VD->getTypeSourceInfo()->getTypeLoc().castAs<PointerTypeLoc>();
+  ASSERT_EQ(SM.getFileOffset(TL.getStarLoc()), Example.point("star"));
+}
+
+TEST(PointerTypeLoc, StarLocBehindSugar) {
+  llvm::Annotations Example(R"c(
+    #define NODEREF __attribute__((noderef))
+    char $1st^* NODEREF _Nonnull $2nd^* var;
+  )c");
+
+  auto AST = tooling::buildASTFromCode(Example.code());
+  SourceManager &SM = AST->getSourceManager();
+  auto &Ctx = AST->getASTContext();
+
+  auto *VD = selectFirst<VarDecl>("vd", match(varDecl(hasName("var")).bind("vd"), Ctx));
+  ASSERT_NE(VD, nullptr);
+
+  auto TL = VD->getTypeSourceInfo()->getTypeLoc().castAs<PointerTypeLoc>();
+  EXPECT_EQ(SM.getFileOffset(TL.getStarLoc()), Example.point("2nd"));
+
+  // Cast intermediate TypeLoc to make sure the structure matches expectations.
+  auto InnerPtrTL = TL.getPointeeLoc().castAs<AttributedTypeLoc>()
+    .getNextTypeLoc().castAs<MacroQualifiedTypeLoc>()
+    .getNextTypeLoc().castAs<AttributedTypeLoc>()
+    .getNextTypeLoc().castAs<PointerTypeLoc>();
+  EXPECT_EQ(SM.getFileOffset(InnerPtrTL.getStarLoc()), Example.point("1st"));
+}
+
 TEST(CXXFunctionalCastExpr, SourceRange) {
   RangeVerifier<CXXFunctionalCastExpr> Verifier;
   Verifier.expectRange(2, 10, 2, 14);
   EXPECT_TRUE(Verifier.match(
       "int foo() {\n"
       "  return int{};\n"
       "}",
       cxxFunctionalCastExpr(), Lang_CXX11));