Page MenuHomePhabricator

InstCombine: don't assume malloc will never return nullptr
AbandonedPublic

Authored by t.p.northover on Nov 14 2018, 8:49 AM.

Details

Reviewers
None
Summary

We've got some code that tries to eliminate unused allocations by inspecting their uses. Unfortunately one of the uses it checks is an icmp, where the code currently assumes that an allocation will never return nullptr. This isn't correct. Most obviously out of memory errors are signalled by nullptr on many systems, but even when the kernel is willing to oversubscribe physical RAM malloc will return nullptr for a large enough single request.

In fact, I believe only certain variants of the C++ operator new never return nullptr (because they throw in error cases). So this patch limits that particular icmp check to the cases where the allocation function is known to be safe.

Diff Detail

Repository
rL LLVM

Event Timeline

t.p.northover created this revision.Nov 14 2018, 8:49 AM

I don't think any of your testcases actually indicate a bug.

Yes, the compiler isn't allowed to assume the libc implementation of malloc doesn't return null, but that's not what's happening here. Instead, we're substituting in our own implementation of malloc, and we can make additional guarantees about that implementation. (This is similar to https://reviews.llvm.org/D53362 , except we don't actually allocate any memory because we prove the allocation is dead.)

t.p.northover abandoned this revision.Nov 20 2018, 5:26 AM

Interesting; I (obviously) hadn't considered that interpretation of what's going on, or I'd have mentioned it. I think it just about holds water, so thanks for setting me straight!

It might be worth adding a comment to the code to make this more clear.

Good idea. I committed a comment in r347653.