[llvm-commits] [llvm] r162131 - /llvm/trunk/lib/CodeGen/StackProtector.cpp

[Chad Rosier]

On Aug 20, 2012, at 4:10 PM, Bill Wendling wrote:

> On Aug 20, 2012, at 3:25 PM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>> On Aug 20, 2012, at 11:21 AM, Bill Wendling wrote:
>> 
>>> On Aug 19, 2012, at 11:57 PM, Duncan Sands <baldrick at free.fr> wrote:
>>> 
>>>> Hi Bill,
>>>> 
>>>>>>> Implement stack protectors for structures with character arrays in them.
>>>>>>> <rdar://problem/10545247>
>>>>>>> 
>>>>>>> +      // If we're on a non-Darwin platform or we're inside of a structure, don't
>>>>>>> +      // add stack protectors unless the array is a character array.
>>>>>>> +      if (InStruct || !Trip.isOSDarwin())
>>>>>>> +          return false;
>>>>>> 
>>>>>> why do you treat Darwin specially here?
>>>>> 
>>>>> Because we want to protect all array types on Darwin, not just char arrays. However, the default gcc behavior is to protect just char arrays with stack protectors. We don't want to have non-Darwin platforms diverge from gcc's behavior here.
>>>> 
>>>> shouldn't this be governed by an option that clang (or whoever) sets?
>>>> 
>>> I have no real opinion on this. Though it's probably better to use Asan and Tsan for real security...
>>> 
>>>>>> Also, why do you care whether there is
>>>>>> an array or not, isn't it only the amount of memory allocated by the alloca that
>>>>>> matters?
>>>>>> 
>>>>> No. We are protecting only arrays with stack protectors.
>>>> 
>>>> OK, thanks for explaining.
>>>> 
>>>> By the way can you please take a look at PR9673 which is just teaching clang to
>>>> turn gcc's ssp-buffer-size option into LLVM's -stack-protector-buffer-size
>>>> (llvm-gcc has done this for ages).  This is important for Debian since there are
>>>> nearly 100 Debian packages which use ssp-buffer-size and are compiled with
>>>> -Werror.  They fail to compile due to clang warning "unused argument during
>>>> compilation" about ssp-buffer-size.
>>>> 
>> 
>> Duncan,
>> How do the attached patches look?
>> 
>> <pr9673.clang.patch><pr9673.llvm.patch>
>> 
>> Still needs a llvm codegen test, but should be working..
>> 
> Chad,
> 
> Make sure that other front-ends -- llc and tools/lto -- are setting this parameter too.

Revised llvm patch attached.  Unmodified clang patch attached for reference.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr9673.clang.patch
Type: application/octet-stream
Size: 4319 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120820/6e8e14eb/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr9673.llvm.patch
Type: application/octet-stream
Size: 4168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120820/6e8e14eb/attachment-0001.obj>
-------------- next part --------------


Thanks, Bill.

 Chad

> 
> -bw
> 
>