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

Reply via email to