llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Akash Manna (akash-manna-sky) <details> <summary>Changes</summary> Fixes #<!-- -->170139 Fixes #<!-- -->120082 `__atomic_always_lock_free` and `__atomic_is_lock_free` check the size with `CharUnits::isPowerOfTwo()`, which is also true for zero. So a size of 0 was treated as a lock-free size, and when the pointer argument was a constant we went on to test its alignment against that size, which builds `llvm::Align(0)` and asserts. Without assertions nothing crashed, but the answer depended on how the pointer was written: `__atomic_always_lock_free(0, p)` was true for a null pointer or `&obj` and false for `(void *)4`. Both constant evaluators now require the size to be positive, so 0 is handled like any other size that isn't lock-free, such as 3: `__atomic_always_lock_free` is false, and `__atomic_is_lock_free` and `__c11_atomic_is_lock_free` are not folded and end up as the runtime call. GCC does the same. I left `CharUnits::isPowerOfTwo()` alone because ObjC property codegen relies on it accepting a zero-sized ivar. The positive-size check also rejects `2^63`, which wraps to a negative `CharUnits` and used to be reported as lock-free. LLM tools (Gemini 3.1 Pro, not Agent) were used for this contribution. I've reviewed, built, and tested the change myself before pushing to GitHub. --- Full diff: https://github.com/llvm/llvm-project/pull/229031.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+2) - (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+2-2) - (modified) clang/lib/AST/ExprConstant.cpp (+1-1) - (modified) clang/test/AST/ByteCode/atomic.c (+14) - (modified) clang/test/CodeGen/atomic-ops.c (+16) - (modified) clang/test/Sema/atomic-ops.c (+12) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index bf194e87f76a6..6ff60826e5648 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -605,6 +605,8 @@ features cannot lower the translation-unit ABI level; inside a member function call synthesized by ``__builtin_invoke``. (#GH185241) - Fixed a crash in ``__builtin_dump_struct`` when ``-Werror`` promotes format warnings to errors. (#GH211943) +- Fixed a crash when `__atomic_always_lock_free` or `__atomic_is_lock_free` is + called with a size of zero. (#GH170139, #GH120082) - Fixed wrong code generation in `__builtin_clear_padding` wherein the wrong bits of the following types were cleared: `_BitInt`, struct bitfields, and packed boolean vectors. (#GH215809), (#GH216063), (#GH224033) diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp index 5ea3cc6bd2920..48b896e2ccece 100644 --- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp +++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp @@ -1181,7 +1181,7 @@ static bool interp__builtin_atomic_lock_free(InterpState &S, CodePtr OpPC, // Check power-of-two. CharUnits Size = CharUnits::fromQuantity(SizeVal); - if (Size.isPowerOfTwo()) { + if (Size.isPositive() && Size.isPowerOfTwo()) { // Check against inlining width. unsigned InlineWidthBits = S.getASTContext().getTargetInfo().getMaxAtomicInlineWidth(); @@ -1239,7 +1239,7 @@ static bool interp__builtin_c11_atomic_is_lock_free(InterpState &S, return false; CharUnits Size = CharUnits::fromQuantity(SizeVal); - if (Size.isPowerOfTwo()) { + if (Size.isPositive() && Size.isPowerOfTwo()) { // Check against inlining width. unsigned InlineWidthBits = S.getASTContext().getTargetInfo().getMaxAtomicInlineWidth(); diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 34c3e39807c7f..98ed45ceaa0fa 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -18084,7 +18084,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, // Check power-of-two. CharUnits Size = CharUnits::fromQuantity(SizeVal.getZExtValue()); - if (Size.isPowerOfTwo()) { + if (Size.isPositive() && Size.isPowerOfTwo()) { // Check against inlining width. unsigned InlineWidthBits = Info.Ctx.getTargetInfo().getMaxAtomicInlineWidth(); diff --git a/clang/test/AST/ByteCode/atomic.c b/clang/test/AST/ByteCode/atomic.c index a93787ccd1e02..ff85c98a0f7b7 100644 --- a/clang/test/AST/ByteCode/atomic.c +++ b/clang/test/AST/ByteCode/atomic.c @@ -65,9 +65,23 @@ _Static_assert(!__atomic_always_lock_free(4, (void*)2), ""); _Static_assert(!__atomic_always_lock_free(4, (void*)-2), ""); _Static_assert(__atomic_always_lock_free(4, (void*)4), ""); _Static_assert(__atomic_always_lock_free(4, (void*)-4), ""); +_Static_assert(!__atomic_always_lock_free(2, (void*)1), ""); +_Static_assert(__atomic_always_lock_free(2, (void*)2), ""); +_Static_assert(!__atomic_always_lock_free(8, (void*)4), ""); +_Static_assert(__atomic_always_lock_free(8, (void*)8), ""); _Static_assert(__atomic_always_lock_free(1, "string"), ""); _Static_assert(!__atomic_always_lock_free(2, "string"), ""); _Static_assert(__atomic_always_lock_free(2, (int[2]){}), ""); void dummyfn(); _Static_assert(__atomic_always_lock_free(2, dummyfn) || 1, ""); + +// GH170139, GH120082 +_Static_assert(!__atomic_always_lock_free(0, 0), ""); +_Static_assert(!__atomic_always_lock_free(0, 1), ""); // both-error {{incompatible integer to pointer conversion}} +_Static_assert(!__atomic_always_lock_free(0, (void*)4), ""); +_Static_assert(!__atomic_always_lock_free(0, &ai), ""); +_Static_assert(__atomic_is_lock_free(0, 0), ""); // both-error {{not an integral constant expression}} +_Static_assert(__atomic_is_lock_free(0, (void*)4), ""); // both-error {{not an integral constant expression}} +_Static_assert(__atomic_is_lock_free(0, &ai), ""); // both-error {{not an integral constant expression}} +_Static_assert(__c11_atomic_is_lock_free(0), ""); // both-error {{not an integral constant expression}} diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c index 8716d3aeb9516..c2318c8512956 100644 --- a/clang/test/CodeGen/atomic-ops.c +++ b/clang/test/CodeGen/atomic-ops.c @@ -370,6 +370,22 @@ int lock_free(struct Incomplete *incomplete) { return __c11_atomic_is_lock_free(sizeof(_Atomic(int))); } +int gh170139(void) { + // CHECK-LABEL: @gh170139 + + // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 noundef 0, ptr noundef null) + __atomic_is_lock_free(0, 0); + + // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 noundef 0, ptr noundef null) + __c11_atomic_is_lock_free(0); + + // CHECK-NOT: call + __atomic_always_lock_free(0, 0); + + // CHECK: ret i32 0 + return __atomic_always_lock_free(0, 0); +} + // Tests for atomic operations on big values. These should call the functions // defined here: // http://gcc.gnu.org/wiki/Atomic/GCCMM/LIbrary#The_Library_interface diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c index a3bc8d2ec75e7..cebbc78ea2262 100644 --- a/clang/test/Sema/atomic-ops.c +++ b/clang/test/Sema/atomic-ops.c @@ -147,6 +147,18 @@ _Static_assert(__atomic_always_lock_free(2, (int[2]){}), ""); void dummyfn(); _Static_assert(__atomic_always_lock_free(2, dummyfn) || 1, ""); +// GH170139, GH120082 +_Static_assert(!__atomic_always_lock_free(0, 0), ""); +_Static_assert(!__atomic_always_lock_free(0, 1), ""); // expected-error {{incompatible integer to pointer conversion}} +_Static_assert(!__atomic_always_lock_free(0, (void*)4), ""); +_Static_assert(!__atomic_always_lock_free(0, &i32), ""); +_Static_assert(__atomic_is_lock_free(0, 0), ""); // expected-error {{not an integral constant expression}} +_Static_assert(__atomic_is_lock_free(0, (void*)4), ""); // expected-error {{not an integral constant expression}} +_Static_assert(__atomic_is_lock_free(0, &i32), ""); // expected-error {{not an integral constant expression}} +_Static_assert(__c11_atomic_is_lock_free(0), ""); // expected-error {{not an integral constant expression}} +_Static_assert(!__atomic_always_lock_free(SIZE_MAX / 2 + 1, 0), ""); +_Static_assert(__c11_atomic_is_lock_free(SIZE_MAX / 2 + 1), ""); // expected-error {{not an integral constant expression}} + #define _AS1 __attribute__((address_space(1))) `````````` </details> https://github.com/llvm/llvm-project/pull/229031 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
