[llvm] r246449 - Repress sanitization on User dtor. Modify msan macros for applying attribute

Mon Aug 31 14:16:41 PDT 2015

As a result of off-line discussion, I rolled back the commit (rL246484) for
the following reasons:
- the commit only suppresses an error identified by use-after-destroy
sanitization
- there is already a bug filed (PR24578) that identifies this issue
- the codebase stays cleaner by keeping these 'TODO' type annotations
within the bug tracking system
- the builds of llvm do not execute with use-after-dtor enabled by default,
so there is no current noisy failure to repress

We (+rsmith) discussed possible resolutions for this bug:
- keeping the suppression
- moving the attributes accessed by operator delete to the memory allocated
during operator new, to ensure that their lifetime extended beyond
destruction
- leaving it as is

On Mon, Aug 31, 2015 at 1:08 PM, Evgenii Stepanov <eugenis at google.com>
wrote:

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

-- 
- Naomi Musgrave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150831/0466025a/attachment.html>