Skip to content

Conversation

@AaronBallman
Copy link
Collaborator

When filling out the type locations for a declarator, we handled atomic types and we handled noderef types, but we didn't handle atomic noderef types.

Fixes #116124

@AaronBallman AaronBallman added c c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid labels Nov 14, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2024

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

When filling out the type locations for a declarator, we handled atomic types and we handled noderef types, but we didn't handle atomic noderef types.

Fixes #116124


Full diff: https://github.com/llvm/llvm-project/pull/116237.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaType.cpp (+27-24)
  • (modified) clang/test/Frontend/noderef.cpp (+6)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3fc275b528d215..0c29e99c72481c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -553,6 +553,8 @@ Bug Fixes in This Version
   the unsupported type instead of the ``register`` keyword (#GH109776).
 - Fixed a crash when emit ctor for global variant with flexible array init (#GH113187).
 - Fixed a crash when GNU statement expression contains invalid statement (#GH113468).
+- Fixed a failed assertion when using ``__attribute__((noderef))`` on an
+  ``_Atomic``-qualified type (#GH116124).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index dd7c438cbd33ec..4fac71ba095668 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5837,6 +5837,30 @@ static void fillMatrixTypeLoc(MatrixTypeLoc MTL,
   llvm_unreachable("no matrix_type attribute found at the expected location!");
 }
 
+static void fillAtomicQualLoc(AtomicTypeLoc ATL, const DeclaratorChunk &Chunk) {
+  SourceLocation Loc;
+  switch (Chunk.Kind) {
+  case DeclaratorChunk::Function:
+  case DeclaratorChunk::Array:
+  case DeclaratorChunk::Paren:
+  case DeclaratorChunk::Pipe:
+    llvm_unreachable("cannot be _Atomic qualified");
+
+  case DeclaratorChunk::Pointer:
+    Loc = Chunk.Ptr.AtomicQualLoc;
+    break;
+
+  case DeclaratorChunk::BlockPointer:
+  case DeclaratorChunk::Reference:
+  case DeclaratorChunk::MemberPointer:
+    // FIXME: Provide a source location for the _Atomic keyword.
+    break;
+  }
+
+  ATL.setKWLoc(Loc);
+  ATL.setParensRange(SourceRange());
+}
+
 namespace {
   class TypeSpecLocFiller : public TypeLocVisitor<TypeSpecLocFiller> {
     Sema &SemaRef;
@@ -6223,6 +6247,9 @@ namespace {
     void VisitExtVectorTypeLoc(ExtVectorTypeLoc TL) {
       TL.setNameLoc(Chunk.Loc);
     }
+    void VisitAtomicTypeLoc(AtomicTypeLoc TL) {
+      fillAtomicQualLoc(TL, Chunk);
+    }
     void
     VisitDependentSizedExtVectorTypeLoc(DependentSizedExtVectorTypeLoc TL) {
       TL.setNameLoc(Chunk.Loc);
@@ -6237,30 +6264,6 @@ namespace {
   };
 } // end anonymous namespace
 
-static void fillAtomicQualLoc(AtomicTypeLoc ATL, const DeclaratorChunk &Chunk) {
-  SourceLocation Loc;
-  switch (Chunk.Kind) {
-  case DeclaratorChunk::Function:
-  case DeclaratorChunk::Array:
-  case DeclaratorChunk::Paren:
-  case DeclaratorChunk::Pipe:
-    llvm_unreachable("cannot be _Atomic qualified");
-
-  case DeclaratorChunk::Pointer:
-    Loc = Chunk.Ptr.AtomicQualLoc;
-    break;
-
-  case DeclaratorChunk::BlockPointer:
-  case DeclaratorChunk::Reference:
-  case DeclaratorChunk::MemberPointer:
-    // FIXME: Provide a source location for the _Atomic keyword.
-    break;
-  }
-
-  ATL.setKWLoc(Loc);
-  ATL.setParensRange(SourceRange());
-}
-
 static void
 fillDependentAddressSpaceTypeLoc(DependentAddressSpaceTypeLoc DASTL,
                                  const ParsedAttributesView &Attrs) {
diff --git a/clang/test/Frontend/noderef.cpp b/clang/test/Frontend/noderef.cpp
index 68342a8e6467b8..048f80cd809474 100644
--- a/clang/test/Frontend/noderef.cpp
+++ b/clang/test/Frontend/noderef.cpp
@@ -183,3 +183,9 @@ int *const_cast_check(NODEREF const int *x) {
 const int *const_cast_check(NODEREF int *x) {
   return const_cast<const int *>(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
 }
+
+namespace GH116124 {
+// This declaration would previously cause a failed assertion.
+int *_Atomic a __attribute__((noderef)); // expected-note {{declared here}}
+int x = *a;                              // expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}

@github-actions
Copy link

github-actions bot commented Nov 14, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 402efa733c64bd20b54dbc5b7057868cbb938d07 588ba0911c69591ccfaf9d7edbb9521952ec7af5 --extensions cpp -- clang/lib/Sema/SemaType.cpp clang/test/Frontend/noderef.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 4fac71ba09..d218496b97 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6247,9 +6247,7 @@ namespace {
     void VisitExtVectorTypeLoc(ExtVectorTypeLoc TL) {
       TL.setNameLoc(Chunk.Loc);
     }
-    void VisitAtomicTypeLoc(AtomicTypeLoc TL) {
-      fillAtomicQualLoc(TL, Chunk);
-    }
+    void VisitAtomicTypeLoc(AtomicTypeLoc TL) { fillAtomicQualLoc(TL, Chunk); }
     void
     VisitDependentSizedExtVectorTypeLoc(DependentSizedExtVectorTypeLoc TL) {
       TL.setNameLoc(Chunk.Loc);

Putting the attribute after the declarator has different behavior than
putting it before or on the type itself; the expected diagnostics were
not triggered.

This was caught by precommit CI. My local testing did not catch it
because I was accidentally testing noderef.c instead of noderef.cpp.

This removes the expected diagnostics and just leaves in the "does not
crash" test.
@AaronBallman AaronBallman merged commit 76bb963 into llvm:main Nov 14, 2024
8 of 9 checks passed
@AaronBallman AaronBallman deleted the aballman-gh116124 branch November 14, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category crash-on-valid

Projects

None yet

3 participants