On Tue, 5 Nov 2024, Jakub Jelinek wrote: > 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). > > > 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.
I'd instead like to see -fallocation-dce be honored for malloc/free pairs as well - the default for C could be different from C++ here but I don't see a good reason to distinguish both? Or do we want to treat malloc/free from C++ different than new/delete pairs? > 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. I don't think the code easily allows for this. I did expect the function call case to force the controlling predicate to survive though. Btw, I agree that the patch doesn't make the current situation worse and for strict conformance we should have an option to turn off the optimization. Restricting it for some cases but not others doesn't make much sense IMO. Richard. > Then all those conformance tests would just work, but hopefully all the > important cases could be still optimized away. > > BTW, the DECL_IS_REPLACEABLE_OPERATOR patch has been committed already. > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
