-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[Clang] Be less strict about diagnosing null pointer dereference. #149648
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
[Clang] Be less strict about diagnosing null pointer dereference. #149648
Conversation
In llvm#143667, we made constant evaluation fail on `*null_ptr`, as this is UB. However, `&(*(foo*)0)` seems to be a common pattern, which made llvm#143667 too disruptive. So instead of failing the evaluation, we note the UB, which let clang recovers when checking for constant initialization. Fixes llvm#149500
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesIn #143667, we made constant evaluation fail on So instead of failing the evaluation, we note the UB, which let clang recovers when checking for constant initialization. Fixes #149500 Full diff: https://github.com/llvm/llvm-project/pull/149648.diff 4 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index cfc4729be4184..e07dd317cfb3b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -9346,9 +9346,12 @@ bool LValueExprEvaluator::VisitUnaryDeref(const UnaryOperator *E) {
// [C++26][expr.unary.op]
// If the operand points to an object or function, the result
// denotes that object or function; otherwise, the behavior is undefined.
- return Success &&
- (!E->getType().getNonReferenceType()->isObjectType() ||
- findCompleteObject(Info, E, AK_Dereference, Result, E->getType()));
+ // Because &(*(type*)0) is a common pattern, we do not fail the evaluation
+ // immediately.
+ if (!Success || !E->getType().getNonReferenceType()->isObjectType())
+ return Success;
+ return !!findCompleteObject(Info, E, AK_Dereference, Result, E->getType()) ||
+ Info.noteUndefinedBehavior();
}
bool LValueExprEvaluator::VisitUnaryReal(const UnaryOperator *E) {
diff --git a/clang/test/AST/ByteCode/const-eval.c b/clang/test/AST/ByteCode/const-eval.c
index c8651a744f969..eab14c08ec809 100644
--- a/clang/test/AST/ByteCode/const-eval.c
+++ b/clang/test/AST/ByteCode/const-eval.c
@@ -51,8 +51,6 @@ struct s {
};
EVAL_EXPR(19, ((int)&*(char*)10 == 10 ? 1 : -1));
-// ref-error@-1 {{expression is not an integer constant expression}} \
-// ref-note@-1 {{dereferencing a null pointer}}
#ifndef NEW_INTERP
EVAL_EXPR(20, __builtin_constant_p(*((int*) 10)));
diff --git a/clang/test/Sema/const-eval.c b/clang/test/Sema/const-eval.c
index 87c21120e7c5d..11cc7fbc0feb3 100644
--- a/clang/test/Sema/const-eval.c
+++ b/clang/test/Sema/const-eval.c
@@ -41,9 +41,6 @@ struct s {
};
EVAL_EXPR(19, ((int)&*(char*)10 == 10 ? 1 : -1));
-// expected-error@-1 {{not an integer constant expression}} \
-// expected-note@-1 {{dereferencing a null pointer is not allowed in a constant expression}}
-
EVAL_EXPR(20, __builtin_constant_p(*((int*) 10)));
@@ -153,3 +150,7 @@ struct PR35214_X {
int PR35214_x;
int PR35214_y = ((struct PR35214_X *)&PR35214_x)->arr[1]; // expected-error {{not a compile-time constant}}
int *PR35214_z = &((struct PR35214_X *)&PR35214_x)->arr[1]; // ok, &PR35214_x + 2
+
+
+int * GH149500_p = &(*(int *)0x400);
+static const void *GH149500_q = &(*(const struct sysrq_key_op *)0);
diff --git a/clang/test/SemaCXX/constant-expression-cxx14.cpp b/clang/test/SemaCXX/constant-expression-cxx14.cpp
index 182c0d01141ff..1743e0e3ac4b5 100644
--- a/clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -1445,3 +1445,8 @@ static_assert(test_member_null(), "");
}
}
+
+namespace GH149500 {
+ unsigned int * p = &(*(unsigned int *)0x400);
+ static const void *q = &(*(const struct sysrq_key_op *)0);
+}
|
clang/lib/AST/ExprConstant.cpp
Outdated
// immediately. | ||
if (!Success || !E->getType().getNonReferenceType()->isObjectType()) | ||
return Success; | ||
return !!findCompleteObject(Info, E, AK_Dereference, Result, E->getType()) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really prefer !!
over (bool)
? Or is this to avoid using a C cast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the small nit, LGTM
/cherry-pick c43f828 |
Failed to cherry-pick: c43f828 https://github.com/llvm/llvm-project/actions/runs/16391072188 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
||
namespace GH149500 { | ||
unsigned int * p = &(*(unsigned int *)0x400); | ||
static const void *q = &(*(const struct sysrq_key_op *)0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also nullptr
test?
|
||
|
||
int * GH149500_p = &(*(int *)0x400); | ||
static const void *GH149500_q = &(*(const struct sysrq_key_op *)0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a test w/ NULL
?
In #143667, we made constant evaluation fail on
*null_ptr
, as this is UB. However,&(*(foo*)0)
seems to be a common pattern, which made #143667 too disruptive.So instead of failing the evaluation, we note the UB, which let clang recovers when checking for constant initialization.
Fixes #149500