similar to: [LLVMdev] load widening conflicts with AddressSanitizer

On 12/16/11 12:24 PM, Kostya Serebryany wrote: > Hello, > > We've just got a bug report from Mozilla folks about AddressSanitizer > false positive with -O2. > Turns out there is a conflict between load widening and AddressSanitizer. > > Simple reproducer: > % cat load_widening.c&& echo =========&& clang -O2 -c load_widening.c -flto&&

On Fri, Dec 16, 2011 at 12:19 PM, John Criswell <criswell at illinois.edu> wrote: > On 12/16/11 12:24 PM, Kostya Serebryany wrote: > > Hello, > > We've just got a bug report from Mozilla folks about AddressSanitizer false > positive with -O2. > Turns out there is a conflict between load widening and AddressSanitizer. > > Simple reproducer: > > % cat

On Fri, Dec 16, 2011 at 12:37 PM, Eli Friedman <eli.friedman at gmail.com>wrote: > On Fri, Dec 16, 2011 at 12:19 PM, John Criswell <criswell at illinois.edu> > wrote: > > On 12/16/11 12:24 PM, Kostya Serebryany wrote: > > > > Hello, > > > > We've just got a bug report from Mozilla folks about AddressSanitizer > false > > positive with

Dear All, I think adding metadata and expecting transforms to repect it is a bad idea. It is just too easy for someone who does not know about the metadata to add a transform that ignores it. As for SAFECode, I think we have one of several options for handling load-widening. The most obvious one is to have a pass that just boosts the allocation size of any alloca with an align 16 attribute;

On Wed, Dec 28, 2011 at 12:40 PM, Criswell, John T <criswell at illinois.edu>wrote: > Dear All, > > I think adding metadata and expecting transforms to repect it is a bad > idea. It is just too easy for someone who does not know about the metadata > to add a transform that ignores it. > > As for SAFECode, I think we have one of several options for handling >

________________________________ From: Kostya Serebryany [kcc at google.com] Sent: Wednesday, December 28, 2011 2:46 PM To: Criswell, John T Cc: llvmdev at cs.uiuc.edu Subject: Re: [LLVMdev] load widening conflicts with AddressSanitizer On Wed, Dec 28, 2011 at 12:40 PM, Criswell, John T <criswell at illinois.edu<mailto:criswell at illinois.edu>> wrote: Dear All, I think adding

Hi, [resurrecting an old mail thread about AddressSanitizer false positive caused by load widening] Once the Attribute::AddressSafety is set by clang (a separate patch), fixing this bug may look as simple as this: --- lib/Analysis/MemoryDependenceAnalysis.cpp (revision 148708) +++ lib/Analysis/MemoryDependenceAnalysis.cpp (working copy) @@ -323,6 +323,14 @@

On Mon, Dec 19, 2011 at 4:27 PM, Chris Lattner <clattner at apple.com> wrote: > > On Dec 17, 2011, at 7:40 AM, Rafael Ávila de Espíndola wrote: > > > On 16/12/11 08:46 PM, Chris Lattner wrote: > >> I'm not opposed to disabling this transformation when asan is on, we > just need a clean way to express this in the IR. > > > > Could clang be aware of

On Dec 17, 2011, at 7:40 AM, Rafael Ávila de Espíndola wrote: > On 16/12/11 08:46 PM, Chris Lattner wrote: >> I'm not opposed to disabling this transformation when asan is on, we just need a clean way to express this in the IR. > > Could clang be aware of asan being on and introduce a "please don't > widen" metadata on local variable accesses? Yes, "we

On Tue, Jan 24, 2012 at 10:23:06AM +0100, Duncan Sands wrote: > Hi Kostya, > > > [resurrecting an old mail thread about AddressSanitizer false positive caused by > > load widening] > > > > Once the Attribute::AddressSafety is set by clang (a separate patch), fixing > > this bug may look as simple as this: > > I don't get the point of an attribute.

Hi Kostya, > [resurrecting an old mail thread about AddressSanitizer false positive caused by > load widening] > > Once the Attribute::AddressSafety is set by clang (a separate patch), fixing > this bug may look as simple as this: I don't get the point of an attribute. There's plenty of code out there that does wide loads like this directly (without them being created by

On Dec 16, 2011, at 12:39 PM, Kostya Serebryany wrote: > > Do we consider the above transformation legal? Yes, the transformation is perfectly legal for the normal compiler. > > I would argue that it should not be legal. We don't actually know what > > comes after the 22 byte object. Is it another memory object? A > > memory-mapped I/O device? Unmapped memory?

On Tue, Jan 24, 2012 at 1:23 AM, Duncan Sands <baldrick at free.fr> wrote: > Hi Kostya, > > > [resurrecting an old mail thread about AddressSanitizer false positive > caused by > > load widening] > > > > Once the Attribute::AddressSafety is set by clang (a separate patch), > fixing > > this bug may look as simple as this: > > Hi Duncan, >

On Fri, Dec 16, 2011 at 6:08 PM, Kostya Serebryany <kcc at google.com> wrote: > In this case, we have an array of 22 bytes which is 16-aligned. >> I suspect that load widening changed the alignment of alloca instruction >> to make the transformation legal. Right? >> Can we change the load widening algorithm to also change the size of >> alloca instruction to be

> > This is a good question. Would it be possible for ASan to do its > instrumentation earlier? > It would be possible but undesirable. First, asan blows up the IR and running asan early will increase the compile-time. Second, asan greatly benefits from all optimizations running before it because it needs to instrument less memory accesses. It actually benefits from load widening

On Fri, Dec 16, 2011 at 2:35 PM, Chris Lattner <clattner at apple.com> wrote: > On Dec 16, 2011, at 2:27 PM, Kostya Serebryany wrote: > > This is a good question. Would it be possible for ASan to do its >> instrumentation earlier? >> > > It would be possible but undesirable. > First, asan blows up the IR and running asan early will increase the >

Hi Chandler, > On Tue, Jan 24, 2012 at 1:36 PM, Kostya Serebryany <kcc at google.com > <mailto:kcc at google.com>> wrote: > > ASAN *can* be modified this way (it will actually make instrumentation ~10% > cheaper). > But this mode will miss some bugs that the current mode finds. > I've seen at least a couple of such *real* bugs. > >

Hi Kostya, > On Tue, Jan 24, 2012 at 1:23 AM, Duncan Sands <baldrick at free.fr > <mailto:baldrick at free.fr>> wrote: > > Hi Kostya, > > > [resurrecting an old mail thread about AddressSanitizer false positive > caused by > > load widening] > > > > Once the Attribute::AddressSafety is set by clang (a separate

On Dec 16, 2011, at 2:27 PM, Kostya Serebryany wrote: > This is a good question. Would it be possible for ASan to do its instrumentation earlier? > > It would be possible but undesirable. > First, asan blows up the IR and running asan early will increase the compile-time. > Second, asan greatly benefits from all optimizations running before it because it needs to instrument

On Tue, Jan 24, 2012 at 1:36 PM, Kostya Serebryany <kcc at google.com> wrote: > ASAN *can* be modified this way (it will actually make instrumentation > ~10% cheaper). > But this mode will miss some bugs that the current mode finds. > I've seen at least a couple of such *real* bugs. > > And these bugs are not only about exploitability, but also about > correctness.