[PATCH] D108123: [MemoryBuiltins] Mark user defined delete as nobuiltin

[guopeilin via Phabricator via llvm-commits]

guopeilin added a comment.

In D108123#2946803 <https://reviews.llvm.org/D108123#2946803>, @lebedev.ri wrote:

> In D108123#2946790 <https://reviews.llvm.org/D108123#2946790>, @guopeilin wrote:
>
>> In D108123#2946738 <https://reviews.llvm.org/D108123#2946738>, @lebedev.ri wrote:
>>
>>> In D108123#2946733 <https://reviews.llvm.org/D108123#2946733>, @guopeilin wrote:
>>>
>>>> In D108123#2946665 <https://reviews.llvm.org/D108123#2946665>, @lebedev.ri wrote:
>>>>
>>>>> This test already passes without the code change: https://godbolt.org/z/48TnbeKj1
>>>>
>>>> The store is not expected, which will become unreachable after SimplifyCFG.
>>>
>>> Right, but that is the correct outcome.
>>> Why is it not UB to free an `undef` (a.k.a. any random pointer)?
>>> It is as per the current langref: https://alive2.llvm.org/ce/z/jBqe-E
>>>
>>> Also, i would like to note that the patch's description doesn't mention that this wants to make `free(undef)` non-UB.
>>
>> The key point is that function `free` can be overload, take this test case as an example, we can overload function `_ZdlPv` with an empty body. So the result becomes that we call a `free` function which actually does not free anything.  Absolutely this is not a way that `free an undef`.
>> So it is better to delete an instruction that calling an empty function with an undefined argument.
>> I will update this test case later, also the description.
>
> Thanks, but this still does not sound good to me.
> If you want to change semantics of `free()`/`_ZdlPv()` in such a way,
> what you need to do is to mark it as `nobuiltin`: https://godbolt.org/z/a5P3TzG7T

Hi, I update a new patch that fix the attribute `nobuiltin`
The source code is like following:

  int a = 0;
  void operator delete(void *p) __attribute__((noinline));
  
  void operator delete(void *p)
  {
    a = 1;
  }
  
  int test() {
    int *p = new int(2);
    a = *p;
    delete p;
    return a;
  }
  
  int main(){
    a = test();
    return 1;
  }

If we find a delete/free function with extact definition, then we can not treat them as `builtin` function any more.


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

https://reviews.llvm.org/D108123