[llvm-commits] [PATCH] Disable an ASan-unsafe optimization with address_safety attribute

[Nick Lewycky]

Evgeniy Stepanov wrote:
> Right. Thanks for noticing :)

Index: test/Transforms/InstCombine/address-safety.ll
===================================================================
--- test/Transforms/InstCombine/address-safety.ll	(revision 0)
+++ test/Transforms/InstCombine/address-safety.ll	(revision 0)
@@ -0,0 +1,32 @@
+; RUN: opt -instcombine -S < %s | FileCheck %s
+; http://llvm.org/bugs/show_bug.cgi?id=12047

The format for the bug line is "; PR12047" and the test runner will 
(used to?) detect that and print that bug number when the test failed. 
Note that llvm.org/PR12047 gets you to the bug quickly.

+%struct.B = type { i32 }
+%struct.A = type { i8*, i8*, i8* }
+%struct.S = type { %struct.A, %struct.B }

I just want to make sure I understand this. Your test indicates a 
machine with 32-bit pointers. %struct.A will be padded out to 4x 
ptrsize, but inside %struct.S, %struct.B could fit inside struct.A's 
padding. Is that what's going on?

Your comment is straight-forward:

+    // If the cast type has alloc size larger than store size, uses of 
the new
+    // alloc may end up accessing the alignment padding. This is ok in 
a regular
+    // build, but Address Safety analysis tools may report false warnings.

but I don't understand how it matches the test. Instcombine transforms 
the alloca %struct.S into an alloca i96, which is only 3x ptrsize. Is 
that transform *ever* legal? Wasn't the code accessing the i32 inside 
%struct.B legally before, and the illegally afterwards?

(A related question for me is how does -scalarrepl handle the similar 
case, though it's hard to produce a testcase it doesn't just reduce away 
entirely.)

Nick

>
> On Wed, Feb 29, 2012 at 4:27 PM, Jia Liu<proljc at gmail.com>  wrote:
>> Hi Evgeniy,
>>
>> On Wed, Feb 29, 2012 at 8:15 PM, Evgeniy Stepanov
>> <eugeni.stepanov at gmail.com>  wrote:
>>> Hi,
>>>
>>> please review this fix for bug 12047.
>>>
>>> One of InstCombine optimizations can replace an allocation of type T
>>> with an allocation of type S, where S has a smaller storage size, but
>>> the same allocation size. This is ok for regular builds, but
>>> incompatible with address safety analysis tools like AddressSanitizer.
>>>
>>> This patch disables this optimization in functions with address_safety
>>> attribute.
>>
>> Did you forgot patch in your mail?
>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits