[Clang] Throw error when calling atomic with pointer to zero size object #91057
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Hendrik Hübner (HendrikHuebner) ChangesWhen an atomic builtin is called with a pointer to an object of size zero, an arithmetic exception gets thrown because there is a modulo operation with the objects size in codegen. This is described here: #90330 I have added a check to SemaChecking.cpp, which throws an error if this is the case. Full diff: https://github.com/llvm/llvm-project/pull/91057.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index e456ec2cac461..74d96a8cbe0dc 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -330,6 +330,9 @@ def warn_atomic_op_misaligned : Warning<
"; the expected alignment (%0 bytes) exceeds the actual alignment (%1 bytes)">,
InGroup<AtomicAlignment>;
+def err_atomic_op_size_zero : Error<
+ "First argument cannot be a pointer to an object of size zero">;
+
def warn_atomic_op_oversized : Warning<
"large atomic operation may incur "
"significant performance penalty"
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..f91ab833b4fc7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -40,6 +40,7 @@
#include "clang/Basic/AddressSpaces.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
@@ -8497,7 +8498,9 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
<< 0 << AdjustedNumArgs << static_cast<unsigned>(Args.size())
<< /*is non object*/ 0 << ExprRange;
return ExprError();
- } else if (Args.size() > AdjustedNumArgs) {
+ }
+
+ if (Args.size() > AdjustedNumArgs) {
Diag(Args[AdjustedNumArgs]->getBeginLoc(),
diag::err_typecheck_call_too_many_args)
<< 0 << AdjustedNumArgs << static_cast<unsigned>(Args.size())
@@ -8544,6 +8547,13 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
}
}
+ // pointer to object of size zero is not allowed
+ if (Context.getTypeInfoInChars(AtomTy).Width.isZero()) {
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_size_zero)
+ << Ptr->getSourceRange();
+ return ExprError();
+ }
+
// For an arithmetic operation, the implied arithmetic must be well-formed.
if (Form == Arithmetic) {
// GCC does not enforce these rules for GNU atomics, but we do to help catch
diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 1d36667d6cf40..97da2582312bb 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -639,6 +639,38 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, -1); // expected-warning {{memory order argument to atomic operation is invalid}}
}
+struct Z {
+ char z[];
+};
+
+void zeroSizeArgError(struct Z *a, struct Z *b, struct Z *c) {
+ __atomic_exchange(b, b, c, memory_order_relaxed); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_exchange(b, b, c, memory_order_acq_rel); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_exchange(b, b, c, memory_order_acquire); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_exchange(b, b, c, memory_order_consume); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_exchange(b, b, c, memory_order_release); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_exchange(b, b, c, memory_order_seq_cst); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_relaxed); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_acq_rel); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_acquire); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_consume); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_release); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_load(a, b, memory_order_seq_cst); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_relaxed); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_acq_rel); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_acquire); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_consume); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_release); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_store(a, b, memory_order_seq_cst); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_relaxed, memory_order_relaxed); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_acq_rel, memory_order_acq_rel); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_acquire, memory_order_acquire); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_consume, memory_order_consume); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_release, memory_order_release); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+ __atomic_compare_exchange(a, b, c, 0, memory_order_seq_cst, memory_order_seq_cst); // expected-error {{First argument cannot be a pointer to an object of size zero}}
+
+}
+
void nullPointerWarning(void) {
volatile _Atomic(int) vai;
_Atomic(int) ai;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
HendrikHuebner
force-pushed
the
dev/clang
branch
2 times, most recently
from
770e7c5
to
d587410
Compare
HendrikHuebner
force-pushed
the
dev/clang
branch
3 times, most recently
from
9a0cb15
to
c6a28f1
Compare
|
Alright, I implemented the changes you suggested. There may have been an easier way to modify the diagnostic though. |
|
The diagnostics around atomic builtins/operations could be cleaned up a bit, but that can be a separate pr. |
|
@HendrikHuebner One thing that is still missing though is a release note. |
|
@HendrikHuebner Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
…96374) This patch fixes the crash when pointers to incomplete type are passed to atomic builtins such as `__atomic_load`. `ASTContext::getTypeInfoInChars` assumes that the argument type is a complete type, so I added a check to eliminate cases where incomplete types gets passed to this function Relevant PR: #91057 Fixes #96289
…lvm#96374) This patch fixes the crash when pointers to incomplete type are passed to atomic builtins such as `__atomic_load`. `ASTContext::getTypeInfoInChars` assumes that the argument type is a complete type, so I added a check to eliminate cases where incomplete types gets passed to this function Relevant PR: llvm#91057 Fixes llvm#96289
When an atomic builtin is called with a pointer to an object of size zero, an arithmetic exception gets thrown because there is a modulo operation with the objects size in codegen. This is described here: #90330
I have added a check to SemaChecking.cpp, which throws an error if this is the case.