> On Tue, Nov 05, 2024 at 04:47:20PM +0100, Jan Hubicka wrote:
> > > POSIX semantics for malloc involve errno.
> >
> > So if I can check errno to see if malloc failed, I guess even our
> > current behaviour of optimizing away paired malloc+free calls provided
> > that the return value is unused is problematic under POSIX same way as
> > the proposed patch.
>
> I think the unconditional malloc+free case is fine.
> errno may be randomly modified by any function which doesn't fail, except
> for a few special cases (like atoi etc.).
> So, one can't really rely on a specific value in errno after a malloc/free
> pair unless one actually checks the return value of malloc and relies on
> errno only if it returned NULL.
> Unless it is something where one relies that the malloc must have definitely
> failed and in code in between the malloc and free saves the errno value
> (because after free it is certainly undefined again).
man page promises that free preserves errno. But such a code would be
weird.
>
> > The attached patch adds code to track size of allocated block and
> > disable the transformation when the block is not known to be smaller
> > then half of the address space by ranger. We can do the runtime check
> > discussed on the top of that.
>
> Thinking about this some more, I think we should just add -fno-malloc-dce
> option and do it even if ranges don't guarantee it won't be half of AS or
> more, that is really just a special case and not too different from
> doing 3 PTRDIFF_MAX - 10 allocations and expecting at least one of those
> will fail, etc.
> glibc tests can use -fno-malloc-dce, or add some optimization barrier
> between the allocation and deallocation which makes compiler think that the
> allocation is actually used.
This would be my preferred variant. I think -mno-malloc-dce is good flag
to have even though glibc test probably should disable malloc builtin to
be sure that no other transformations are done.
>
> Or the other option is decide not based on the size range, but what the
> if (!ptr) code actually does, allow jumping around the freeing, allow
> __builtin_unreachable, don't allow anything else.
> Then all those conformance tests would just work, but hopefully all the
> important cases could be still optimized away.
I would expect someting
if (!ptr)
output_error_message_and_exit ()
to be common pattern, like it happens in GCC after xmalloc is inlined.
So parsing what if (!ptr) block does seems bit difficult.
>
> BTW, the DECL_IS_REPLACEABLE_OPERATOR patch has been committed already.
Thanks I noticed that! I will add testcase that nothrow variant of new
is now optimized away. We still need the __builtion_operator_new bits
so libstdc++ will use it by default.
Honza
>
> Jakub
>
