llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Aditya Medhane (flash1729) <details> <summary>Changes</summary> `__has_unique_object_representations(_Atomic(int))` returns false even though `_Atomic(int)` has the same object representation as `int`. Atomic types were never handled: they fail the trivial-copyability check up front (they aren't scalar types), and `_Atomic T` has been on the FIXME list at the end of the function since the original implementation. This is also the root cause of the false positive pinned in the `bugprone-suspicious-memory-comparison` tests since [D89651](https://reviews.llvm.org/D89651). Handle `AtomicType` explicitly: `_Atomic(T)` has unique object representations iff `T` does and the atomic's size wasn't rounded up to a power of two (see `getTypeInfo`), since the rounding bytes are padding. The branch goes before the trivial-copyability check, which atomics always fail; triviality is judged on the value type instead. Adds trait asserts to `type-traits.cpp` and flips the tidy FIXME test to expect no diagnostic. This is groundwork for a follow-up PR adding a Sema warning for `memcmp` on types without unique object representations, so the known false positive needed fixing first. --- Full diff: https://github.com/llvm/llvm-project/pull/212768.diff 4 Files Affected: - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.c (+1-3) - (modified) clang/docs/ReleaseNotes.md (+6) - (modified) clang/lib/AST/ASTContext.cpp (+8-1) - (modified) clang/test/SemaCXX/type-traits.cpp (+10) ``````````diff diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.c index d3ecffec9a781..1f8bd7bd9f1ad 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.c @@ -286,9 +286,7 @@ struct AtomicMember { }; void Test_AtomicMember(void) { - // FIXME: this is a false positive as the list of objects with unique object - // representations is incomplete. + // _Atomic(int) has the same object representation as int: no warning. struct AtomicMember a, b; memcmp(&a, &b, sizeof(struct AtomicMember)); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct AtomicMember' which does not have a unique object representation; consider comparing the members of the object manually } diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 80e770a40838a..20b37f466b84f 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -327,6 +327,12 @@ features cannot lower the translation-unit ABI level; - Fixed a crash when classifying a call to a builtin with dependent arguments, such as when the call is used as an `auto` non-type template argument. +- `__has_unique_object_representations` now returns `true` for `_Atomic` types + whose object representation is identical to that of their value type, such + as `_Atomic(int)`. Atomic types whose size is rounded up to a power of two + (adding padding bits) continue to report `false`. This also fixes a false + positive in the `bugprone-suspicious-memory-comparison` clang-tidy check. + #### Bug Fixes to Attribute Support - The `counted_by`/`counted_by_or_null` diagnostic that rejects a pointer whose diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 2228811546c0f..6aea6cfcf25ca 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -3062,6 +3062,14 @@ bool ASTContext::hasUniqueObjectRepresentations( "hasUniqueObjectRepresentations should not be called with an " "incomplete type"); + // _Atomic(T) shares T's object representation unless its size was rounded + // up to a power of two, in which case the extra bytes are padding. Atomic + // types are never trivially copyable, so (9.1) is judged on the value type. + if (const auto *AT = Ty->getAs<AtomicType>()) + return getTypeSize(AT) == getTypeSize(AT->getValueType()) && + hasUniqueObjectRepresentations(AT->getValueType(), + CheckIfTriviallyCopyable); + // (9.1) - T is trivially copyable... if (CheckIfTriviallyCopyable && !Ty.isTriviallyCopyableType(*this)) return false; @@ -3102,7 +3110,6 @@ bool ASTContext::hasUniqueObjectRepresentations( // FIXME: More cases to handle here (list by rsmith): // vectors (careful about, eg, vector of 3 foo) // _Complex int and friends - // _Atomic T // Obj-C block pointers // Obj-C object pointers // and perhaps OpenCL's various builtin types (pipe, sampler_t, event_t, diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp index ff74461308fb1..246da619fe046 100644 --- a/clang/test/SemaCXX/type-traits.cpp +++ b/clang/test/SemaCXX/type-traits.cpp @@ -3504,6 +3504,16 @@ static_assert(__has_unique_object_representations(const int *), "as are pointers static_assert(__has_unique_object_representations(volatile int *), "as are pointers"); static_assert(__has_unique_object_representations(const volatile int *), "as are pointers"); +static_assert(__has_unique_object_representations(_Atomic(int)), "layout-identical atomics are"); +static_assert(!__has_unique_object_representations(_Atomic(float)), "value type is not unique"); +struct AtomicReprThreeChars { char a, b, c; }; +static_assert(!__has_unique_object_representations(_Atomic(AtomicReprThreeChars)), + "atomic size rounded up to a power of two adds padding"); +struct AtomicReprMember { _Atomic(int) x; }; +static_assert(__has_unique_object_representations(AtomicReprMember), "atomic member, no padding"); +struct AtomicReprPadded { char c; _Atomic(int) x; }; +static_assert(!__has_unique_object_representations(AtomicReprPadded), "padding before atomic member"); + class C {}; using FP = int (*)(int); using PMF = int (C::*)(int); `````````` </details> https://github.com/llvm/llvm-project/pull/212768 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
