[PATCH] D96349: [instcombine] Exploit UB implied by nofree attributes

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D96349#2552491 <https://reviews.llvm.org/D96349#2552491>, @reames wrote:

> In D96349#2551984 <https://reviews.llvm.org/D96349#2551984>, @jdoerfert wrote:
>
>> LGTM. We should probably add the following negative test case:
>>
>>   ; Freeing in a nonfree function is fine if the effect is invisible to the outside
>>   define void @test16() nofree {
>>     i8* %foo = call i8* @malloc(i32 1)
>>     call void @free(i8* %foo)
>>     ret void
>>   }
>
> Er, no.  That's not okay.  Either by the implementation in this patch, or by my reading of the LangRef.  The langref says "This function attribute indicates that the function does not, directly or indirectly, call a memory-deallocation function (free, for example). "  There's no exception there for memory allocated in scope, nor should there be.

Right, we want to tweak the lang ref. Generally, I believe this is is one of those cases where it is "as if" it doesn't free. I mean, you can't tell it does from the outside. Imagine a backend that realizes the stack allocation is too big and puts it on the heap with a proper free. If the function was nofree before it would not be anymore even though we might have "thought so" during the compilation. Similarly, a `writenone` function can write to some alloca without it being a problem (IMHO).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96349/new/

https://reviews.llvm.org/D96349