<div dir="ltr">As a result of off-line discussion, I rolled back the commit (<span style="font-size:12.8000001907349px">rL246484)</span> for the following reasons:<div>- the commit only suppresses an error identified by use-after-destroy sanitization<br><div>- there is already a bug filed (PR24578) that identifies this issue</div></div><div>- the codebase stays cleaner by keeping these 'TODO' type annotations within the bug tracking system</div><div>- the builds of llvm do not execute with use-after-dtor enabled by default, so there is no current noisy failure to repress</div><div><br></div><div>We (+rsmith) discussed possible resolutions for this bug:</div><div>- keeping the suppression</div><div>- moving the attributes accessed by operator delete to the memory allocated during operator new, to ensure that their lifetime extended beyond destruction</div><div>- leaving it as is</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 31, 2015 at 1:08 PM, Evgenii Stepanov <span dir="ltr"><<a href="mailto:eugenis@google.com" target="_blank">eugenis@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, but with this suppression we can do it earlier. Also, this<br>
after-destruction access looks quite intentional, and we may decide<br>
not to fix it at all. Anyway, it sounds like it may take a while.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On Mon, Aug 31, 2015 at 12:36 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
> Right, but sooner or later should probably mean "when we're clean" and not<br>
> at other times? Or am I missing something?<br>
><br>
> -eric<br>
><br>
> On Mon, Aug 31, 2015 at 12:35 PM Evgenii Stepanov <<a href="mailto:eugenis@google.com">eugenis@google.com</a>><br>
> wrote:<br>
>><br>
>> We have the options, and they are off by default at this point. But we<br>
>> want to enable them on the sanitizer bootstrap bot sooner or later.<br>
>><br>
>><br>
>> On Mon, Aug 31, 2015 at 12:26 PM, Naomi Musgrave <<a href="mailto:nmusgrave@google.com">nmusgrave@google.com</a>><br>
>> wrote:<br>
>> > I'm not entirely sure if the compile and runtime options for<br>
>> > use-after-dtor<br>
>> > poisoning will be fully integrated into MSan or not when I leave. To<br>
>> > clarify, are you reccommending leaving these as distinct options?<br>
>> ><br>
>> > On Mon, Aug 31, 2015 at 12:22 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >><br>
>> >><br>
>> >> On Mon, Aug 31, 2015 at 10:36 AM Naomi Musgrave <<a href="mailto:nmusgrave@google.com">nmusgrave@google.com</a>><br>
>> >> wrote:<br>
>> >>><br>
>> >>> I have looked repeatedly into a fix for this bug, with no progress<br>
>> >>> yet.<br>
>> >>> I'm currently repressing it to be able to proceed with investigating<br>
>> >>> other<br>
>> >>> bugs.<br>
>> >><br>
>> >><br>
>> >> OK. That seems reasonable.<br>
>> >><br>
>> >>><br>
>> >>> If I have not resolved this issue before my internship is over, I<br>
>> >>> think<br>
>> >>> it may be better to file bugs against these issues.<br>
>> >>><br>
>> >><br>
>> >> *nod* Might be worth an option to disable the feature in general so we<br>
>> >> don't need the source annotations?<br>
>> >><br>
>> >> -eric<br>
>> >><br>
>> >>><br>
>> >>> Note: commit reverted in rL246450 due to it breaking a flaky build.<br>
>> >>><br>
>> >>> On Mon, Aug 31, 2015 at 9:23 AM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
>> >>> wrote:<br>
>> >>>><br>
>> >>>><br>
>> >>>>><br>
>> >>>>> Summary: In response to bug 24578, reported against failing LLVM<br>
>> >>>>> test.<br>
>> >>>>><br>
>> >>>><br>
>> >>>> FWIW we generally just say "PR24578"<br>
>> >>>><br>
>> >>>> So, hrm, how long do we expect the "workaround" annotation to be in<br>
>> >>>> the<br>
>> >>>> source?<br>
>> >>>><br>
>> >>>> -eric<br>
>> >>>><br>
>> >>>><br>
>> >>>>><br>
>> >>>>> Reviewers: chandlerc, rsmith, eugenis<br>
>> >>>>><br>
>> >>>>> Subscribers: llvm-commits<br>
>> >>>>><br>
>> >>>>> Differential Revision: <a href="http://reviews.llvm.org/D12335" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12335</a><br>
>> >>>>><br>
>> >>>>> Modified:<br>
>> >>>>> llvm/trunk/include/llvm/IR/User.h<br>
>> >>>>> llvm/trunk/include/llvm/Support/Compiler.h<br>
>> >>>>> llvm/trunk/lib/IR/Metadata.cpp<br>
>> >>>>> llvm/trunk/lib/IR/User.cpp<br>
>> >>>>><br>
>> >>>>> Modified: llvm/trunk/include/llvm/IR/User.h<br>
>> >>>>> URL:<br>
>> >>>>><br>
>> >>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/User.h?rev=246449&r1=246448&r2=246449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/User.h?rev=246449&r1=246448&r2=246449&view=diff</a><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> ==============================================================================<br>
>> >>>>> --- llvm/trunk/include/llvm/IR/User.h (original)<br>
>> >>>>> +++ llvm/trunk/include/llvm/IR/User.h Mon Aug 31 10:57:40 2015<br>
>> >>>>> @@ -72,8 +72,7 @@ protected:<br>
>> >>>>> void growHungoffUses(unsigned N, bool IsPhi = false);<br>
>> >>>>><br>
>> >>>>> public:<br>
>> >>>>> - ~User() override {<br>
>> >>>>> - }<br>
>> >>>>> + ~User() override {}<br>
>> >>>>> /// \brief Free memory allocated for User and Use objects.<br>
>> >>>>> void operator delete(void *Usr);<br>
>> >>>>> /// \brief Placement delete - required by std, but never called.<br>
>> >>>>><br>
>> >>>>> Modified: llvm/trunk/include/llvm/Support/Compiler.h<br>
>> >>>>> URL:<br>
>> >>>>><br>
>> >>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=246449&r1=246448&r2=246449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=246449&r1=246448&r2=246449&view=diff</a><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> ==============================================================================<br>
>> >>>>> --- llvm/trunk/include/llvm/Support/Compiler.h (original)<br>
>> >>>>> +++ llvm/trunk/include/llvm/Support/Compiler.h Mon Aug 31 10:57:40<br>
>> >>>>> 2015<br>
>> >>>>> @@ -323,10 +323,12 @@<br>
>> >>>>> #if __has_feature(memory_sanitizer)<br>
>> >>>>> # define LLVM_MEMORY_SANITIZER_BUILD 1<br>
>> >>>>> # include <sanitizer/msan_interface.h><br>
>> >>>>> +# define LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE<br>
>> >>>>> __attribute__((no_sanitize_memory))<br>
>> >>>>> #else<br>
>> >>>>> # define LLVM_MEMORY_SANITIZER_BUILD 0<br>
>> >>>>> # define __msan_allocated_memory(p, size)<br>
>> >>>>> # define __msan_unpoison(p, size)<br>
>> >>>>> +# define LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE<br>
>> >>>>> #endif<br>
>> >>>>><br>
>> >>>>> /// \macro LLVM_ADDRESS_SANITIZER_BUILD<br>
>> >>>>><br>
>> >>>>> Modified: llvm/trunk/lib/IR/Metadata.cpp<br>
>> >>>>> URL:<br>
>> >>>>><br>
>> >>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=246449&r1=246448&r2=246449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=246449&r1=246448&r2=246449&view=diff</a><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> ==============================================================================<br>
>> >>>>> --- llvm/trunk/lib/IR/Metadata.cpp (original)<br>
>> >>>>> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Aug 31 10:57:40 2015<br>
>> >>>>> @@ -401,7 +401,9 @@ void *MDNode::operator new(size_t Size,<br>
>> >>>>> return Ptr;<br>
>> >>>>> }<br>
>> >>>>><br>
>> >>>>> -void MDNode::operator delete(void *Mem) {<br>
>> >>>>> +// Repress memory sanitization, due to use-after-destroy by<br>
>> >>>>> operator<br>
>> >>>>> +// delete. Bug report 24578 identifies this issue.<br>
>> >>>>> +LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE void MDNode::operator delete(void<br>
>> >>>>> *Mem) {<br>
>> >>>>> MDNode *N = static_cast<MDNode *>(Mem);<br>
>> >>>>> size_t OpSize = N->NumOperands * sizeof(MDOperand);<br>
>> >>>>> OpSize = RoundUpToAlignment(OpSize, llvm::alignOf<uint64_t>());<br>
>> >>>>><br>
>> >>>>> Modified: llvm/trunk/lib/IR/User.cpp<br>
>> >>>>> URL:<br>
>> >>>>><br>
>> >>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/User.cpp?rev=246449&r1=246448&r2=246449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/User.cpp?rev=246449&r1=246448&r2=246449&view=diff</a><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> ==============================================================================<br>
>> >>>>> --- llvm/trunk/lib/IR/User.cpp (original)<br>
>> >>>>> +++ llvm/trunk/lib/IR/User.cpp Mon Aug 31 10:57:40 2015<br>
>> >>>>> @@ -118,7 +118,9 @@ void *User::operator new(size_t Size) {<br>
>> >>>>> // User operator delete Implementation<br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> //===----------------------------------------------------------------------===//<br>
>> >>>>><br>
>> >>>>> -void User::operator delete(void *Usr) {<br>
>> >>>>> +// Repress memory sanitization, due to use-after-destroy by<br>
>> >>>>> operator<br>
>> >>>>> +// delete. Bug report 24578 identifies this issue.<br>
>> >>>>> +LLVM_NO_SANITIZE_MEMORY_ATTRIBUTE void User::operator delete(void<br>
>> >>>>> *Usr) {<br>
>> >>>>> // Hung off uses use a single Use* before the User, while other<br>
>> >>>>> subclasses<br>
>> >>>>> // use a Use[] allocated prior to the user.<br>
>> >>>>> User *Obj = static_cast<User *>(Usr);<br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> _______________________________________________<br>
>> >>>>> llvm-commits mailing list<br>
>> >>>>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>> >>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>> >>><br>
>> >>><br>
>> >>><br>
>> >>><br>
>> >>> --<br>
>> >>> - Naomi Musgrave<br>
>> >>><br>
>> ><br>
>> ><br>
>> ><br>
>> > --<br>
>> > - Naomi Musgrave<br>
>> ><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">- Naomi Musgrave<div><br></div></div></div>
</div>