[llvm-commits] AddressSanitizer, first patch

[Kostya Serebryany]

FYI
I've added noreturn attribute to the __asan_report_error call. It doesn't
cause compile problems in cpu2006 any more.
The good news is that the slowdown (compared to assembly with ud2) on
cpu2006 ranges between 1% and 8%.
And we may recover from it if __asan_report_error does not cause stack frame
to be created in leaf functions.

--kcc


On Fri, Sep 9, 2011 at 12:48 PM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Thu, Sep 8, 2011 at 5:38 PM, Eric Christopher <echristo at apple.com>wrote:
>
>>
>> On Sep 8, 2011, at 5:14 PM, Kostya Serebryany wrote:
>>
>>
>>
>> On Thu, Sep 8, 2011 at 4:43 PM, Eric Christopher <echristo at apple.com>wrote:
>>
>>>
>>> On Sep 8, 2011, at 4:35 PM, Chandler Carruth wrote:
>>>
>>> > FYI,
>>> >
>>> > I've looked again at this patch recently, and talked to several others
>>> about it...
>>> >
>>> > I think there are some big issues with the implementation, bit I think
>>> they could probably be addressed. The one that I see the most is the x86
>>> assembly just being dumped as a blob. I think that really needs to be
>>> resolved before this can move forward.
>>> >
>>> > It seems like this should be do-able in some way with the llvm.trap
>>> intrinsic. If anything, we might need another intrinsic or an extension of
>>> llvm.trap that allows feeding the SIGILL signal handler the data arguments
>>> it needs; I'd need to read up on exactly what is required for that to figure
>>> out how best to represent that in IR, but it shouldn't be that hard.
>>> >
>>> > That said, maybe to improve the discussion of the patch and make
>>> reviewing it easier, you could just switch to emitting the call to the
>>> runtime library function in all cases? That should be a strictly simpler
>>> patch, and then how to improve performance by avoiding the runtime call can
>>> be a followup.
>>> >
>>>
>>> This encompasses most of my concerns with the patch.
>>>
>>> > Maybe others could comment on other implementation issues? I'm sadly
>>> not an expert at LLVM...
>>>
>>> In a glance I didn't see a lot, but the other bits were somewhat hiding
>>> it. How about when we get a new patch we can take a bit more of a look at
>>> it.
>>>
>>
>> I dropped the assembly part and updated the patch at
>> http://codereview.appspot.com/4867059/ (also in attachment).
>> We can figure out how to make it more efficient later (something
>> like llvm.trap intrinsic with a parameter).
>>
>> This patch is 570 LOC plus few lines of trivial changes and a test.
>> It implements only basic functionality: detection of out-of-bounds and
>> use-after-free in heap.
>>
>> The complete version of LLVM pass is ~1000 LOC (
>> http://code.google.com/p/address-sanitizer/source/browse/trunk/llvm/AddressSanitizer.cpp
>> ).
>> It also handles out-of-bounds for stack and global objects and (recently
>> added) stack use-after-return bugs.
>>
>>
>> Some somewhat trivial issues:
>>
>> a) Please go ahead and hide the non-user visible flags.
>>
> done
>
>>
>> b) Not a big fan of this style of avoid 80-col wrap:
>>
>> +Instruction *AddressSanitizer::generateCrashCode(
>> +    IRBuilder<> &IRB, Value *Addr, int TelltaleValue) {
>>
> done
>
>>
>> c) +  // which will be lowered to a cusom assembly.
>>
>> s/cusom/custom
>>
>> d)
>>
>> +  if (TypeSize != 8 && TypeSize != 16 &&
>> +      TypeSize != 32 && TypeSize != 64 && TypeSize != 128) {
>> +    // TODO(kcc): do something better.
>> +    return;
>> +  }
>>
>> What's the reason behind this?
>>
>
> Changed the comment.
>
>
>>
>> Perhaps not so trivial:
>>
>> a) Where are the asan_* functions? Where are they supposed to live?
>>
>> I'll probably have more, but it's a start and the discussion on the
>> library functions probably should happen sooner rather than later.
>>
>> -eric
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110912/2485ede8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: issue4867059_23001.diff
Type: text/x-patch
Size: 23503 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110912/2485ede8/attachment.bin>