Re: [PATCH v4 2/3] c++: Improve constexpr error for dangling local variables [PR110619]

[Jason Merrill <jason@redhat.com>]

On 7/21/23 01:39, Nathaniel Shead wrote:
> On Thu, Jul 20, 2023 at 11:46:47AM -0400, Jason Merrill wrote:
>> On 7/20/23 05:36, Nathaniel Shead wrote:
>>> Currently, when typeck discovers that a return statement will refer to a
>>> local variable it rewrites to return a null pointer. This causes the
>>> error messages for using the return value in a constant expression to be
>>> unhelpful, especially for reference return values.
>>>
>>> This patch removes this "optimisation".
>>
>> This isn't an optimization, it's for safety, removing a way for an attacker
>> to get a handle on other data on the stack (CWE-562).
>>
>> But I agree that we need to preserve some element of UB for constexpr
>> evaluation to see.
>>
>> Perhaps we want to move this transformation to cp_maybe_instrument_return,
>> so it happens after maybe_save_constexpr_fundef?
> 
> Hm, OK. I can try giving this a go. I guess I should move the entire
> maybe_warn_about_returning_address_of_local function to cp-gimplify.cc
> to be able to detect this? Or is there a better way of marking that a
> return expression will return a reference to a local for this
> transformation? (I guess I can't use whether the warning has been
> surpressed or not because the warning might not be enabled at all.)

You could use a TREE_LANG_FLAG, looks like none of them are used on 
RETURN_EXPR.

> It looks like this warning is raised also by diag_return_locals in
> gimple-ssa-isolate-paths, should the transformation also be made here?

Looks like it already is, in warn_return_addr_local:

>       tree zero = build_zero_cst (TREE_TYPE (val));
>       gimple_return_set_retval (return_stmt, zero);
>       update_stmt (return_stmt);

...but, weirdly, only with -fisolate-erroneous-paths-*, even though it 
isn't isolating anything.  Perhaps there should be another flag for this.

> I note that the otherwise very similar -Wdangling-pointer warning
> doesn't do this transformation either, should that also be something I
> look into fixing here?

With that same flag, perhaps.  I wonder if it would make sense to remove 
the isolate-paths handling of locals in favor of the dangling-pointer 
handling?  I don't know either file much at all.

Jason