[Clang] Fix crash in __builtin_assume_aligned #114217
Conversation
The CodeGen for __builtin_assume_aligned assumes that the first argument is a pointer, so crashes if the int-conversion error is downgraded or disabled. Emit a non-downgradable error if the argument is not a pointer, like we currently do for __builtin_launder. Fixes llvm#110914.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang Author: Oliver Stannard (ostannard) ChangesThe CodeGen for __builtin_assume_aligned assumes that the first argument is a pointer, so crashes if the int-conversion error is downgraded or disabled. Emit a non-downgradable error if the argument is not a pointer, like we currently do for __builtin_launder. Fixes #110914. Full diff: https://github.com/llvm/llvm-project/pull/114217.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 34ff49d7238a7f..67ef5fcae142c2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12271,6 +12271,8 @@ def warn_noderef_to_dereferenceable_pointer : Warning<
def err_builtin_launder_invalid_arg : Error<
"%select{non-pointer|function pointer|void pointer}0 argument to "
"'__builtin_launder' is not allowed">;
+def err_builtin_assume_aligned_invalid_arg : Error<
+ "non-pointer argument to '__builtin_assume_aligned' is not allowed">;
def err_builtin_is_within_lifetime_invalid_arg : Error<
"%select{non-|function }0pointer argument to '__builtin_is_within_lifetime' "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3308b898a5b68f..f6f67895973cc7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5272,8 +5272,11 @@ bool Sema::BuiltinAssumeAligned(CallExpr *TheCall) {
{
ExprResult FirstArgResult =
DefaultFunctionArrayLvalueConversion(FirstArg);
- if (checkBuiltinArgument(*this, TheCall, 0))
+ if (!FirstArgResult.get()->getType()->isPointerType()) {
+ Diag(TheCall->getBeginLoc(), diag::err_builtin_assume_aligned_invalid_arg)
+ << TheCall->getSourceRange();
return true;
+ }
/// In-place updation of FirstArg by checkBuiltinArgument is ignored.
TheCall->setArg(0, FirstArgResult.get());
}
diff --git a/clang/test/Sema/builtin-assume-aligned.c b/clang/test/Sema/builtin-assume-aligned.c
index 33e85578451529..57378a3426524a 100644
--- a/clang/test/Sema/builtin-assume-aligned.c
+++ b/clang/test/Sema/builtin-assume-aligned.c
@@ -74,7 +74,7 @@ int test13(int *a) {
}
int test14(int *a, int b) {
- a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *}}
+ a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{non-pointer argument to '__builtin_assume_aligned' is not allowed}}
}
int test15(int *b) {
|
| @@ -74,7 +74,7 @@ int test13(int *a) { | |||
| } | |||
|
|
|||
| int test14(int *a, int b) { | |||
| a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *}} | |||
| a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{non-pointer argument to '__builtin_assume_aligned' is not allowed}} | |||
There was a problem hiding this comment.
Can we add the test cases from the issue, specifically: #110914 (comment) and #110914 (comment)
We should always include tests that trigger crashes we are fixing to catch possible future regression.
| @@ -5272,8 +5272,11 @@ bool Sema::BuiltinAssumeAligned(CallExpr *TheCall) { | |||
| { | |||
| ExprResult FirstArgResult = | |||
| DefaultFunctionArrayLvalueConversion(FirstArg); | |||
| if (checkBuiltinArgument(*this, TheCall, 0)) | |||
There was a problem hiding this comment.
checkBuiltinArgument should always either produce an expression with the correct type, or error out. If neither is happening, there's something wrong with type-checking.
There was a problem hiding this comment.
Took another look at this... probably this is okay. checkBuiltinArgument isn't actually doing anything useful here, and other places do something similar with DefaultFunctionArrayLvalueConversion().
The comment about "In-place updation of FirstArg by checkBuiltinArgument" should be deleted, though.
|
ping on this we have another regression linked to the original change: #120086 We really should land a fix ASAP, this is now three regression linked to the same change. |
The CodeGen for __builtin_assume_aligned assumes that the first argument is a pointer, so crashes if the int-conversion error is downgraded or disabled. Emit a non-downgradable error if the argument is not a pointer, like we currently do for __builtin_launder.
Fixes #110914.