On Thu, Sep 03, 2020 at 10:53:37PM -0400, Tom Lane wrote: > Noah Misch <n...@leadboat.com> writes: > > We do assume dereferencing NULL would crash, but we also assume this > > optimization doesn't happen: > > > #ifndef REMOVE_MEMCPY > > memcpy(dest, src, n); > > #endif > > if (src) > > pause(); > > > [ gcc believes the if-test is unnecessary ] > > > So yes, it would be reasonable to adopt -fno-delete-null-pointer-checks > > and/or > > remove -fno-sanitize=nonnull-attribute from buildfarm member thorntail.
> If there actually are places where this is a problem, I think we > need to fix it by doing > > if (n > 0) > memcpy(dest, src, n); > > so that the compiler can no longer assume that src!=NULL even > when n is zero. I'd still leave -fdelete-null-pointer-checks > enabled, because it can make valid and useful optimizations in > other cases. (Besides that, it's far from clear that disabling > that flag would suppress all bad consequences of the assumption > that memcpy's arguments aren't null.) Your proposal is what I had in mind when I wrote "remove -fno-sanitize=nonnull-attribute from buildfarm member thorntail", and I agree it's attractive. In particular, gcc is not likely to be the last compiler to attempt such an optimization, and other compilers may not offer -fno-delete-null-pointer-checks or equivalent. One might argue for -fno-delete-null-pointer-checks in addition, because it would defend against cases that sanitizers miss. I tend to think that's overkill, but maybe not. I suppose one could do an audit, diffing the generated code with and without the option.
