Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[C] Silence unreachable -Watomic-access diagnostics #140064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 15, 2025

Conversation

AaronBallman
Copy link
Collaborator

Accessing the member of a structure or union which is _Atomic-qualified is undefined behavior in C. We currently diagnose that with a warning that defaults to an error. In turn, this means we reject a valid program if the access it not reachable because of the error. e.g.,

if (0)
SomeAtomicStruct.Member = 12; // Was diagnosed

This silences the diagnostic if the member access is not reachable.

Accessing the member of a structure or union which is _Atomic-qualified
is undefined behavior in C. We currently diagnose that with a warning
that defaults to an error. In turn, this means we reject a valid
program if the access it not reachable because of the error. e.g.,

  if (0)
    SomeAtomicStruct.Member = 12; // Was diagnosed

This silences the diagnostic if the member access is not reachable.
@AaronBallman AaronBallman added clang Clang issues not falling into any other category c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" rejects-valid labels May 15, 2025
@llvmbot
Copy link
Member

llvmbot commented May 15, 2025

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Accessing the member of a structure or union which is _Atomic-qualified is undefined behavior in C. We currently diagnose that with a warning that defaults to an error. In turn, this means we reject a valid program if the access it not reachable because of the error. e.g.,

if (0)
SomeAtomicStruct.Member = 12; // Was diagnosed

This silences the diagnostic if the member access is not reachable.


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+13)
  • (modified) clang/lib/Sema/SemaExprMember.cpp (+1-1)
  • (modified) clang/test/Sema/atomic-expr.c (+17)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 31c517338c21f..f927c438c5d80 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -533,6 +533,19 @@ Improvements to Clang's diagnostics
 - A new off-by-default warning ``-Wms-bitfield-padding`` has been added to alert to cases where bit-field
   packing may differ under the MS struct ABI (#GH117428).
 
+- ``-Watomic-access`` no longer fires on unreachable code. e.g.,
+
+  ..code-block:: c
+
+    _Atomic struct S { int a; } s;
+    void func(void) {
+      if (0)
+        s.a = 12; // Previously diagnosed with -Watomic-access, now silenced
+      s.a = 12; // Still diagnosed with -Watomic-access
+      return;
+      s.a = 12; // Previously diagnosed, now silenced
+    }
+
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 053414ff7a1a7..39c162c3b835d 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1385,7 +1385,7 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
   // lvalue. Because this is inherently unsafe as an atomic operation, the
   // warning defaults to an error.
   if (const auto *ATy = BaseType->getAs<AtomicType>()) {
-    S.DiagRuntimeBehavior(OpLoc, nullptr,
+    S.DiagRuntimeBehavior(OpLoc, BaseExpr.get(),
                           S.PDiag(diag::warn_atomic_member_access));
     BaseType = ATy->getValueType().getUnqualifiedType();
     BaseExpr = ImplicitCastExpr::Create(
diff --git a/clang/test/Sema/atomic-expr.c b/clang/test/Sema/atomic-expr.c
index 7e5219dd3f14a..96571e3e68c87 100644
--- a/clang/test/Sema/atomic-expr.c
+++ b/clang/test/Sema/atomic-expr.c
@@ -114,6 +114,23 @@ void func_16(void) {
   (void)sizeof(xp->val);
   (void)sizeof(y.ival);
   (void)sizeof(yp->ival);
+
+  // Also, do not diagnose in unreachable code paths.
+  {
+    if (0) {
+      x.val = 12;
+      xp->val = 12;
+      (void)y.ival;
+      (void)yp->ival;
+    }
+
+    return;
+
+    x.val = 12;
+    xp->val = 12;
+    (void)y.ival;
+    (void)yp->ival;
+  }
 }
 
 // Ensure that we correctly implement assignment constraints from C2x 6.5.16.1.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously going to still be somewhat imperfect, but seems fine to me.

@AaronBallman AaronBallman merged commit 0eb4bd2 into llvm:main May 15, 2025
12 checks passed
@AaronBallman AaronBallman deleted the aballman-atomic-member-access branch May 15, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category rejects-valid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.