| Message ID | 574F2CF7.1000800@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 06/01/2016 02:44 PM, Martin Sebor wrote: >>> The new code in cxx_eval_component_reference diagnoses the following >>> problem that's not detected otherwise: >>> >>> struct S { const S *s; }; >>> >>> constexpr S s = { 0 }; >>> >>> constexpr const void *p = &s.s->s; >> >> Note that this falls under core issue 1530, which has not been resolved. > > I don't quite see the relevance of this issue. It's concerned > with storage in which an object will exist at some point in > the future when its lifetime begins or where it existed in > the past before its lifetime ended. There is no object or > storage at s.s above because s.s is null. True. Unfortunately there isn't any wording about what you can do with the result of indirecting a null pointer or pointer to one-past-the-end of an array. There was some proposed for issue 232, but that hasn't been a high priority. What do you see in the standard currently that makes it undefined? > It might perhaps be okay to accept the expression above as > an extension with the rationale that the offset of the first > member is zero and so its address must be a null pointer, but > it surely wouldn't be okay in the modified test case below > where the second member's offset is non-zero and there may > not be a way to represent that offset as a pointer. Why not? It would be a pointer to address 4 or whatever. > Thanks for the hint. I've made that change and also adjusted > the rest of the patch to avoid passing the nullptr_p around. > It was surprisingly simpler to do than I remembered from my > first attempt back in March. > + if (code == POINTER_PLUS_EXPR && !*non_constant_p > + && tree_int_cst_equal (lhs, null_pointer_node)) > + { > + if (!ctx->quiet) > + error ("arithmetic involving a null pointer in %qE", lhs); > + return t; > + } > + if (TREE_CODE (whole) == INDIRECT_REF > + && integer_zerop (TREE_OPERAND (whole, 0)) > + && !ctx->quiet) > + error ("dereferencing a null pointer in %qE", orig_whole); > + if (TREE_CODE (t) == INTEGER_CST > + && TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE > + && !integer_zerop (t)) > + { > + if (!ctx->quiet) > + error ("arithmetic involving a null pointer in %qE", t); > + } These places should all set *non_constant_p, and the second should return t after doing so. OK with that change. Jason
On 06/01/2016 01:20 PM, Jason Merrill wrote: > On 06/01/2016 02:44 PM, Martin Sebor wrote: >>>> The new code in cxx_eval_component_reference diagnoses the following >>>> problem that's not detected otherwise: >>>> >>>> struct S { const S *s; }; >>>> >>>> constexpr S s = { 0 }; >>>> >>>> constexpr const void *p = &s.s->s; >>> >>> Note that this falls under core issue 1530, which has not been resolved. >> >> I don't quite see the relevance of this issue. It's concerned >> with storage in which an object will exist at some point in >> the future when its lifetime begins or where it existed in >> the past before its lifetime ended. There is no object or >> storage at s.s above because s.s is null. > > True. Unfortunately there isn't any wording about what you can do with > the result of indirecting a null pointer or pointer to one-past-the-end > of an array. There was some proposed for issue 232, but that hasn't > been a high priority. What do you see in the standard currently that > makes it undefined? In N4567, expr.ref, the E1->E2 expression which is equivalent to (*E1).E2, the closest match is paragraph 4.2: If E2 is a non-static data member and the type of E1 is "cq1 vq1 X", and the type of E2 is "cq2 vq2 T", the expression designates the named member of the object designated by the first expression. There is no object, so 4.2 doesn't apply, and since there is no other bullet that would apply, the behavior is undefined. >> It might perhaps be okay to accept the expression above as >> an extension with the rationale that the offset of the first >> member is zero and so its address must be a null pointer, but >> it surely wouldn't be okay in the modified test case below >> where the second member's offset is non-zero and there may >> not be a way to represent that offset as a pointer. > > Why not? It would be a pointer to address 4 or whatever. Because the hardware may simply not have a representation for 4 (or any small integer) in a pointer type. Possibly because it uses that bit pattern for some special mapping and when there is no mapping, using the invalid pointer value traps. Whether or not this is the case is implementation-defined (certainly in C for this reason, and I expect also in C++). For core constant expressions, my impression is that they are deliberately constrained to a fairly narrow guaranteed- to-be-valid subset, which is why things like reinterpret_cast are not allowed. The expression we're discussing basically boils down to a conversion of the offset 4 to a pointer, which is equivalent to the forbidden reinterpret_cast. I agree that the standard text isn't ironclad on this, but I can't think of a useful purpose that allowing constexpr expressions to form invalid addresses would serve even in the (common) case where the hardware does allow it. From a practical point of view, if GCC were to continue to allow it, it would need to track those pointers and diagnose them on their first use, because that is undefined in any case. >> + if (code == POINTER_PLUS_EXPR && !*non_constant_p >> + && tree_int_cst_equal (lhs, null_pointer_node)) >> + { >> + if (!ctx->quiet) >> + error ("arithmetic involving a null pointer in %qE", lhs); >> + return t; >> + } > >> + if (TREE_CODE (whole) == INDIRECT_REF >> + && integer_zerop (TREE_OPERAND (whole, 0)) >> + && !ctx->quiet) >> + error ("dereferencing a null pointer in %qE", orig_whole); > >> + if (TREE_CODE (t) == INTEGER_CST >> + && TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE >> + && !integer_zerop (t)) >> + { >> + if (!ctx->quiet) >> + error ("arithmetic involving a null pointer in %qE", t); >> + } > > These places should all set *non_constant_p, and the second should > return t after doing so. OK with that change. Thanks. I've made the change, but I haven't managed to come up with a test to exercise it. IIUC, the purpose setting non_constant_p while the quiet flag is set is to determine without causing errors that expressions are not valid constant expressions by passing them to __builtin_constant_p, correct? If so, then there must be something wrong somewhere because the test case below fails the static assertion (the first error is expected with the patch): $ cat null.c && /home/msebor/build/gcc-60760/gcc/xgcc -B /home/msebor/build/gcc-60760/gcc -S -Wall -Wextra -Wpedantic -xc++ null.c constexpr int *p = 0; constexpr int *q = p + 1; static_assert (!__builtin_constant_p (p + 1), "<<<"); null.c:2:24: error: arithmetic involving a null pointer in ‘0u’ constexpr int *q = p + 1; ^ null.c:4:1: error: static assertion failed: <<< static_assert (!__builtin_constant_p (p + 1), "<<<"); ^~~~~~~~~~~~~ (I know of at least one bug here, c++/70552, but this one is new.) Martin
On 06/01/2016 10:49 PM, Martin Sebor wrote: > On 06/01/2016 01:20 PM, Jason Merrill wrote: >> On 06/01/2016 02:44 PM, Martin Sebor wrote: >>>>> The new code in cxx_eval_component_reference diagnoses the following >>>>> problem that's not detected otherwise: >>>>> >>>>> struct S { const S *s; }; >>>>> >>>>> constexpr S s = { 0 }; >>>>> >>>>> constexpr const void *p = &s.s->s; >>>> >>>> Note that this falls under core issue 1530, which has not been >>>> resolved. >>> >>> I don't quite see the relevance of this issue. It's concerned >>> with storage in which an object will exist at some point in >>> the future when its lifetime begins or where it existed in >>> the past before its lifetime ended. There is no object or >>> storage at s.s above because s.s is null. >> >> True. Unfortunately there isn't any wording about what you can do with >> the result of indirecting a null pointer or pointer to one-past-the-end >> of an array. There was some proposed for issue 232, but that hasn't >> been a high priority. What do you see in the standard currently that >> makes it undefined? > > In N4567, expr.ref, the E1->E2 expression which is equivalent > to (*E1).E2, the closest match is paragraph 4.2: > > If E2 is a non-static data member and the type of E1 is "cq1 > vq1 X", and the type of E2 is "cq2 vq2 T", the expression > designates the named member of the object designated by the > first expression. > > There is no object, so 4.2 doesn't apply, and since there is > no other bullet that would apply, the behavior is undefined. OK, I'll buy that. >>> + if (code == POINTER_PLUS_EXPR && !*non_constant_p >>> + && tree_int_cst_equal (lhs, null_pointer_node)) >>> + { >>> + if (!ctx->quiet) >>> + error ("arithmetic involving a null pointer in %qE", lhs); >>> + return t; >>> + } >> >>> + if (TREE_CODE (whole) == INDIRECT_REF >>> + && integer_zerop (TREE_OPERAND (whole, 0)) >>> + && !ctx->quiet) >>> + error ("dereferencing a null pointer in %qE", orig_whole); >> >>> + if (TREE_CODE (t) == INTEGER_CST >>> + && TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE >>> + && !integer_zerop (t)) >>> + { >>> + if (!ctx->quiet) >>> + error ("arithmetic involving a null pointer in %qE", t); >>> + } >> >> These places should all set *non_constant_p, and the second should >> return t after doing so. OK with that change. > > Thanks. I've made the change, but I haven't managed to come up > with a test to exercise it. IIUC, the purpose setting > non_constant_p while the quiet flag is set is to determine without > causing errors that expressions are not valid constant expressions > by passing them to __builtin_constant_p, correct? No, __builtin_constant_p allows more things than C++ constant expressions. The purpose of setting *non_constant_p is to cause maybe_constant_value to return its argument unchanged. Jason
PR c++/60760 - arithmetic on null pointers should not be allowed in constant expressions PR c++/71091 - constexpr reference bound to a null pointer dereference accepted gcc/testsuite/ChangeLog: 2016-05-12 Martin Sebor <msebor@redhat.com> PR c++/60760 PR c++/71091 * g++.dg/cpp0x/constexpr-nullptr-2.C: New test. * gcc/testsuite/g++.dg/ubsan/pr63956.C: Adjust. gcc/cp/ChangeLog: 2016-05-12 Martin Sebor <msebor@redhat.com> PR c++/60760 PR c++/71091 * constexpr.c (cxx_eval_binary_expression): Reject invalid arithmetic involving null pointers. (cxx_eval_constant_expression): Same. (cxx_eval_component_reference): Reject null pointer dereferences. diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 482f8af..d1fe276 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1757,6 +1757,13 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, || null_member_pointer_value_p (rhs))) r = constant_boolean_node (!is_code_eq, type); } + if (code == POINTER_PLUS_EXPR && !*non_constant_p + && tree_int_cst_equal (lhs, null_pointer_node)) + { + if (!ctx->quiet) + error ("arithmetic involving a null pointer in %qE", lhs); + return t; + } if (r == NULL_TREE) r = fold_binary_loc (loc, code, type, lhs, rhs); @@ -2097,6 +2104,11 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t, tree whole = cxx_eval_constant_expression (ctx, orig_whole, lval, non_constant_p, overflow_p); + if (TREE_CODE (whole) == INDIRECT_REF + && integer_zerop (TREE_OPERAND (whole, 0)) + && !ctx->quiet) + error ("dereferencing a null pointer in %qE", orig_whole); + if (TREE_CODE (whole) == PTRMEM_CST) whole = cplus_expand_constant (whole); if (whole == orig_whole) @@ -3505,10 +3517,20 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, if (!flag_permissive || ctx->quiet) *overflow_p = true; } + + if (TREE_CODE (t) == INTEGER_CST + && TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE + && !integer_zerop (t)) + { + if (!ctx->quiet) + error ("arithmetic involving a null pointer in %qE", t); + } + return t; } - switch (TREE_CODE (t)) + tree_code tcode = TREE_CODE (t); + switch (tcode) { case RESULT_DECL: if (lval) @@ -3919,7 +3941,6 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, case NOP_EXPR: case UNARY_PLUS_EXPR: { - enum tree_code tcode = TREE_CODE (t); tree oldop = TREE_OPERAND (t, 0); tree op = cxx_eval_constant_expression (ctx, oldop, @@ -3945,15 +3966,26 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, return t; } } - if (POINTER_TYPE_P (type) - && TREE_CODE (op) == INTEGER_CST - && !integer_zerop (op)) - { - if (!ctx->quiet) - error_at (EXPR_LOC_OR_LOC (t, input_location), - "reinterpret_cast from integer to pointer"); - *non_constant_p = true; - return t; + if (POINTER_TYPE_P (type) && TREE_CODE (op) == INTEGER_CST) + { + const char *msg = NULL; + if (integer_zerop (op)) + { + if (!same_type_ignoring_top_level_qualifiers_p (type, + TREE_TYPE (op))) + msg = "invalid conversion involving a null pointer"; + } + else + msg = "reinterpret_cast from integer to pointer"; + + if (msg) + { + if (!ctx->quiet) + error_at (EXPR_LOC_OR_LOC (t, input_location), msg); + + *non_constant_p = true; + return t; + } } if (op == oldop && tcode != UNARY_PLUS_EXPR) /* We didn't fold at the top so we could check for ptr-int diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C new file mode 100644 index 0000000..9ee1316 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C @@ -0,0 +1,191 @@ +// PR c++/60760 - arithmetic on null pointers should not be allowed +// in constant expressions +// PR c++/71091 - constexpr reference bound to a null pointer dereference +// accepted +// { dg-do compile { target c++11 } } +// { dg-additional-options "-Wno-pointer-arith" } + +// Generate a null poiinter. +constexpr int* null () { return 0; } + +// Test case from comment #0 in c++/60760. +namespace PR_60760_comment_0 { + +constexpr int* ptr = nullptr; +constexpr int* ptr2 = ptr + 1; // { dg-error "null pointer|not a constant" } + +} + +// Test case from comment #1 in c++/60760. +namespace PR_60760_comment_1 { + +constexpr int* ptr = nullptr; +constexpr int x = 0; +constexpr int* ptr2 = ptr + x; // Adding zero is valid. +constexpr int* ptr3 = ptr - x; // As is subtracting zero. + +} + +// Test case from c++/71091. +namespace PR_71091 { + +constexpr int *p = 0; +constexpr int &r = *p; // { dg-error "null pointer" } + +} + +// Other testr cases. +namespace C { + +struct S { int a, b[1]; } s; + +constexpr S *p0 = &s; +constexpr S *p1 = nullptr; + +constexpr int *r0 = p1->b; // { dg-error "null pointer|constant expression" } + +} + +namespace D { + +struct A { int i; const A *pa1; const A *pa0; }; + +constexpr A a1 = { 0, 0, 0 }; +constexpr A a2 = { 1, &a1, 0 }; + +constexpr const A *pa2 = &a2; +constexpr int i0 = pa2->i; +constexpr int i1 = pa2->pa1->i; +constexpr int i2 = pa2->pa1->pa0->i; // { dg-error "null pointer|not a constant" } + +constexpr const A *pa3 = &*pa2->pa1->pa0; +constexpr const A *pa4 = pa2->pa1->pa0 + 1; // { dg-error "null pointer|not a constant" } + +constexpr const int *pi0 = &pa2->pa1->pa0->i; // { dg-error "null pointer|not a constant" } + +constexpr const A *pa5 = 0; +constexpr const int *pi1 = &pa5->i; // { dg-error "null pointer|not a constant" } + +} + + +namespace SimpleTests { + +constexpr int* p0 = nullptr; +constexpr int* q0 = p0; +constexpr int* r0 = null (); + +// Adding or subtracting zero from a null pointer is valid in C++. +constexpr int* p1 = p0 + 0; +constexpr int* p2 = p0 - 0; +constexpr int* p3 = 0 + p0; + +// While the text of the C++ standard still doesn't allow it, CWG +// issue 232 implies that dererencing a null pointer is intended +// to be permitted in contexts where the result isn't evaluated. +// For compatibility with C that should at a minimum include +// expressions like &*p that are valid there. +constexpr int* p4 = &*p0; +constexpr int* p5 = p0 + 1; // { dg-error "null pointer|not a constant" } +constexpr int* p6 = 1 + p0; // { dg-error "null pointer|not a constant" } +constexpr int* p7 = p0 - 1; // { dg-error "null pointer|not a constant" } +constexpr int* p8 = &p0 [0]; +constexpr int* p9 = &0 [p0]; + +constexpr int* p10 = null () + 2; // { dg-error "null pointer|not a constant" } +constexpr int* p11 = 3 + null (); // { dg-error "null pointer|not a constant" } +constexpr int* p12 = null () - 4; // { dg-error "null pointer|not a constant" } +constexpr int* p13 = &null ()[4]; // { dg-error "null pointer|not a constant" } +constexpr int* p14 = &3[null ()]; // { dg-error "null pointer|not a constant" } + +constexpr int* q1 = q0 + 0; +constexpr int* q2 = q0 - 0; +constexpr int* q3 = q0 + 1; // { dg-error "null pointer|not a constant" } +constexpr int* q4 = q0 + 2; // { dg-error "null pointer|not a constant" } +constexpr int* q5 = &q0 [0]; + +// Subtracting null pointers from one another is valid. +constexpr int i0 = p0 - (int*)0; +constexpr int i1 = p0 - static_cast<int*>(0); +constexpr int i2 = p0 - (int*)nullptr; +constexpr int i3 = p0 - static_cast<int*>(nullptr); +constexpr int i4 = p0 - p0; +constexpr int i5 = p0 - q0; +constexpr int i6 = p0 - r0; +constexpr int i7 = (int*)0 - p0; +constexpr int i8 = static_cast<int*>(0) - p0; +constexpr int i9 = (int*)nullptr - p0; +constexpr int i10 = static_cast<int*>(nullptr) - p0; +constexpr int i11 = q0 - p0; +constexpr int i12 = r0 - p0; + +} + +namespace IndirectTests { + +struct S { int i, j; struct SA { struct SB { int *pi; } sb; } sa; }; + +constexpr S* ps = (S*)0; + +// Comparing null pointers is valid. +constexpr bool b0 = ps == ps; +constexpr bool b1 = ps != ps; +constexpr bool b2 = ps < ps; +constexpr bool b3 = ps <= ps; +constexpr bool b4 = ps > ps; +constexpr bool b5 = ps >= ps; + +constexpr bool b6 = ps == (S*)0; +constexpr bool b7 = ps != (S*)0; +constexpr bool b8 = ps < (S*)0; +constexpr bool b9 = ps <= (S*)0; +constexpr bool b10 = ps > (S*)0; +constexpr bool b11 = ps >= (S*)0; + +constexpr S* ps1 = ps; +constexpr S* ps2 = ps1; + +// The following aren't diagnosed due to a bug. +// constexpr int* pi0 = &((S*)0)->i; +// constexpr int* pi1 = &((S*)nullptr)->i; + +constexpr int* pj0 = &((S*)0)->j; // { dg-error "null pointer|not a constant" } +constexpr int* pj1 = &((S*)nullptr)->j; // { dg-error "null pointer|not a constant" } + +constexpr int* psi = &ps->i; // { dg-error "null pointer|not a constant" } +constexpr int* psj = &ps->j; // { dg-error "null pointer|not a constant" } + +constexpr int* ps1i = &ps1->i; // { dg-error "null pointer|not a constant" } +constexpr int* ps2i = &ps1->i; // { dg-error "null pointer|not a constant" } + +constexpr int* ps1j = &ps1->j; // { dg-error "null pointer|not a constant" } +constexpr int* ps2j = &ps1->j; // { dg-error "null pointer|not a constant" } + +} + +namespace FunctionTests { + +typedef void Func (); + +// Arithmetic on member function pointers is diagnosed with -Wpointer-arith. +// With constexpr, only zero may be added or subtracted. +constexpr Func *pf0 = 0; +constexpr Func *pf1 = pf0 + 0; // triggers -Wpointer-arith +constexpr Func *pf2 = pf0 - 0; // triggers -Wpointer-arith +constexpr Func *pf3 = 0 + pf0; // triggers -Wpointer-arith +constexpr Func *pf4 = pf0 + 1; // { dg-error "null pointer|not a constant" } +constexpr Func *pf5 = 2 + pf0; // { dg-error "null pointer|not a constant" } +constexpr Func *pf6 = pf0 - 3; // { dg-error "null pointer|not a constant" } + +struct S; +typedef void (S::*MemFuncPtr)(); + +// Arithmetic on member function pointers is rejected with a hard error. +constexpr MemFuncPtr pmf0 = nullptr; +constexpr MemFuncPtr pmf1 = pmf0 + 0; // { dg-error "invalid operands" } +constexpr MemFuncPtr pmf2 = 0 + pmf0; // { dg-error "invalid operands" } +constexpr MemFuncPtr pmf3 = pmf0 + 1; // { dg-error "invalid operands" } +constexpr MemFuncPtr pmf4 = 1 + pmf0; // { dg-error "invalid operands" } +constexpr MemFuncPtr pmf5 = pmf0 - 1; // { dg-error "invalid operands" } + +} diff --git a/gcc/testsuite/g++.dg/ubsan/pr63956.C b/gcc/testsuite/g++.dg/ubsan/pr63956.C index 25db8a4..ac01fa4 100644 --- a/gcc/testsuite/g++.dg/ubsan/pr63956.C +++ b/gcc/testsuite/g++.dg/ubsan/pr63956.C @@ -92,7 +92,7 @@ constexpr int fn6 (const int &a, int b) { if (b != 2) - b = a; // { dg-error "is not a constant expression" } + b = a; return b; } @@ -106,7 +106,7 @@ fn7 (const int *a, int b) constexpr int n1 = 7; constexpr int n2 = fn7 (&n1, 5); -constexpr int n3 = fn7 ((const int *) 0, 8); +constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "null pointer" } constexpr int fn8 (int i)