[PATCH] Implement the no_split_stack attribute.

[Alp Toker]

On 17/05/2014 16:40, Aaron Ballman wrote:
> On Sat, May 17, 2014 at 2:23 AM, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> This is a GNU attribute that allows split stacks to be turned off on a
>> per-function basis.
>>
>> http://reviews.llvm.org/D3817
>>
>> Files:
>>    include/clang/Basic/Attr.td
>>    lib/CodeGen/CGCall.cpp
>>    lib/Sema/SemaDeclAttr.cpp
>>    test/CodeGen/split-stacks.c
>>
>> Index: include/clang/Basic/Attr.td
>> ===================================================================
>> --- include/clang/Basic/Attr.td
>> +++ include/clang/Basic/Attr.td
>> @@ -823,6 +823,12 @@
>>     let Documentation = [Undocumented];
>>   }
>>
>> +def NoSplitStack : InheritableAttr {
>> +  let Spellings = [GNU<"no_split_stack">];
> Should this be GCC instead of GNU? (so it can be spelled
> [[gnu::no_split_stack]] as well.)
>
>> +  let Subjects = SubjectList<[Function]>;
> Should it also apply to templated functions? ObjC methods?
>
>> +  let Documentation = [Undocumented];
> Please add documentation for this attribute.
>
>> +}
>> +
>>   def NonNull : InheritableAttr {
>>     let Spellings = [GCC<"nonnull">];
>>     let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
>> Index: lib/CodeGen/CGCall.cpp
>> ===================================================================
>> --- lib/CodeGen/CGCall.cpp
>> +++ lib/CodeGen/CGCall.cpp
>> @@ -1117,7 +1117,8 @@
>>       FuncAttrs.addAttribute(llvm::Attribute::NoRedZone);
>>     if (CodeGenOpts.NoImplicitFloat)
>>       FuncAttrs.addAttribute(llvm::Attribute::NoImplicitFloat);
>> -  if (CodeGenOpts.EnableSegmentedStacks)
>> +  if (CodeGenOpts.EnableSegmentedStacks &&
>> +      !(TargetDecl && TargetDecl->hasAttr<NoSplitStackAttr>()))
>>       FuncAttrs.addAttribute("split-stack");
>>
>>     if (AttrOnCallSite) {
>> Index: lib/Sema/SemaDeclAttr.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclAttr.cpp
>> +++ lib/Sema/SemaDeclAttr.cpp
>> @@ -4173,6 +4173,9 @@
>>     case AttributeList::AT_NoCommon:
>>       handleSimpleAttribute<NoCommonAttr>(S, D, Attr);
>>       break;
>> +  case AttributeList::AT_NoSplitStack:
>> +    handleSimpleAttribute<NoSplitStackAttr>(S, D, Attr);
>> +    break;
>>     case AttributeList::AT_NonNull:
>>       if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D))
>>         handleNonNullAttrParameter(S, PVD, Attr);
>> Index: test/CodeGen/split-stacks.c
>> ===================================================================
>> --- test/CodeGen/split-stacks.c
>> +++ test/CodeGen/split-stacks.c
>> @@ -5,13 +5,21 @@
>>     return 0;
>>   }
>>
>> +__attribute__((no_split_stack))
>> +int nosplit() {
>> +  return 0;
>> +}
>> +
>>   int main() {
>>     return foo();
>>   }
>>
>> -// CHECK-SEGSTK: define i32 @foo() #0 {
>> -// CHECK-SEGSTK: define i32 @main() #0 {
>> -// CHECK-SEGSTK: #0 = { {{.*}} "split-stack" {{.*}} }
>> +// CHECK-SEGSTK: define i32 @foo() [[SS:#[0-9]+]] {
>> +// CHECK-SEGSTK: define i32 @nosplit() [[NSS:#[0-9]+]] {
>> +// CHECK-SEGSTK: define i32 @main() [[SS]] {
>> +// CHECK-SEGSTK-NOT: [[NSS]] = { {{.*}} "split-stack" {{.*}} }
>> +// CHECK-SEGSTK: [[SS]] = { {{.*}} "split-stack" {{.*}} }
>> +// CHECK-SEGSTK-NOT: [[NSS]] = { {{.*}} "split-stack" {{.*}} }
>>
>>   // CHECK-NOSEGSTK: define i32 @foo() #0 {
>>   // CHECK-NOSEGSTK: define i32 @main() #0 {
> Missing sema tests for the attribute (that it applies only to
> functions, takes no arguments, etc).

Actually I like how Peter's kept the tests concise. It seems reasonable 
to trust the attribute machinery for subject/range checking, similar to 
the way we only test the semantics of type trait expressions, compiler 
builtins etc. where there's existing coverage for standard handling.

Alp.

-- 
http://www.nuanti.com
the browser experts