On 05/31/2016 06:03 PM, Martin Sebor wrote:
On 05/17/2016 01:44 PM, Jason Merrill wrote:
On 05/12/2016 06:34 PM, Martin Sebor wrote:
Attached is a resubmission of the patch for c++/60760 originally
submitted late in the 6.0 cycle along with a patch for c++/67376.
Since c++/60760 was not a regression, it was decided that it
would be safer to defer the fix until after the 6.1.0 release.
While retesting this patch I was happy to notice that it also
fixes another bug: c++/71091 - constexpr reference bound to a null
pointer dereference accepted.
I'm not sure why we need to track nullptr_p through everything. Can't
we set *non_constant_p instead in the places where it's problematic, as
in cxx_eval_binary_expression?
That would have certainly been my preference over adding another
argument to a subset of the functions, making their declarations
inconsistent. Unfortunately, I couldn't come up with a simpler
way to make it work.
FWIW, my first inclination was to make the nullptr_p flag part
of constexpr_ctx and move the other xxx_p arguments there, to
simplify the interface, until I noticed that all the functions
take a const constexpr_ctx*, preventing them to modify it. It
seemed like too much of a change to fix a bug in stage 3, but
I'd be comfortable making it in stage 1 if you think it's
worthwhile (though my preference would be to do in a separate
commit, after fixing this bug).
I understand that the complication comes because of needing to allow
constexpr int *p = &*(int*)0;
but I don't see how cxx_eval_component_reference could come up with a
constant value for the referent of a null pointer, so we already reject
struct A { int i; };
constexpr A* p = nullptr;
constexpr int i = p->i;
In cxx_eval_indirect_ref, we could check !lval and reject the
constant-expression at that point.
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 believe that this is fine under the current wording; only "access"
to a non-static data member is undefined, and "access" is defined as
reading or writing. There has been discussion in the context of UBsan
about making this undefined, but that hasn't been decided yet. But I'm
OK with changing G++ to go in that direction.
cxx_eval_component_reference could check whether 'whole' is a null (or
other invalid) lvalue; for that testcase it's an INDIRECT_REF of 0.
Jason
