On Fri, Jul 21, 2023 at 05:44:51PM -0400, Jason Merrill wrote: > 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 see, thanks. From this I've found that my above patch isn't sufficient anyway, as compiling with -O2 causes the warning to appear twice as the suppression I did wasn't sufficient. As such I'll exclude this patch from the next revision since it's not actually necessary for the problem I was trying to solve, and I'll work on trying to solve this properly a bit later. > > 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 >
