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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

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

> In D96349#2552559 <https://reviews.llvm.org/D96349#2552559>, @jdoerfert wrote:
>
>> 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.
>
> I'm not going to enter a debate on whether the LangRef is right here.  We can do that on llvm-dev or private conversation.  This patch is solely implementing the semantics as currently specified.
>
> Does your LGTM still stand with that understanding?

Yes, this is restrictive enough to be correct either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96349