[Clang] Implement function attribute no_stack_protector.

[manojgupta]

Summary

This attribute tells clang to skip this function from stack protector
when -stack-protector option is passed.
GCC option for this is:
attribute((optimize("no-stack-protector"))) and the
equivalent clang syntax would be: attribute((no_stack_protector))

This is used in Linux kernel to selectively disable stack protector
in certain functions.

[srhines]

Looks fine. Please wait for a more experienced person to review this, however. Thanks for adding this feature.

[aaron.ballman]

Missing Sema tests that the attribute only appertains to functions, accepts no arguments, etc.

include/clang/Basic/Attr.td

1494

This is not a GCC attribute, so this should use the Clang spelling.

However, why spell the attribute this way rather than use the GCC spelling (optimize("no-stack-protector")?

1496

No new undocumented attributes, please.

[manojgupta]

Added docs, Sema test case for the attribute.

[manojgupta]

include/clang/Basic/Attr.td

1494

Thanks, I have changed it to use Clang spelling.

Regarding attribute((optimize("..."))), it is a generic facility in GCC that works for many optimizer flags.
Clang currently does not support this syntax currently instead preferring its own version for some options e.g. -O0.
e.g.

__attribute__((optimize("O0")))  // clang version is __attribute__((optnone))

If we want to support the GCC syntax, future expectation would be support more flags under this syntax. Is that the path we want to take (I do not know the history related to previous syntax decisions but better GCC compatibility will be a nice thing to have)

[aaron.ballman]

Adding in @probinson as he originally added the optnone attribute. Paul, do you recall why you opted (haha, pun totally intended) to implement optnone rather than optimize("O0") from GCC?

include/clang/Basic/Attr.td

1494

The history of optnone predates my involvement with Clang and I've not been able to find the original review thread (I did find the one where I gave my LGTM on the original patch, but that was a resubmission after the original design was signed off).

I'm not keen on attributes that have the same semantics but differ in spelling from attributes supported by GCC unless there's a good reason to deviate. Given that we've already deviated, I'd like to understand why better -- I don't want to push you to implement something we've already decided was a poor design, but I also don't want to accept code if we can instead use syntax that is compatible with GCC.

[rnk]

include/clang/Basic/Attr.td

1494

I do not think we want to pursue supporting generic optimization flags with __attribute__((optimize("..."))).

Part of the reason we only support optnone is that LLVM's pass pipeline is global to the entire module. It's not trivial to enable optimizations for a single function, but it is much easier to have optimization passes skip functions marked optnone.

[aaron.ballman]

include/clang/Basic/Attr.td

1494

Thank you, @rnk, that makes sense to me and seems like a solid reason for this approach.

include/clang/Basic/AttrDocs.td

2746 ↗

(On Diff #145514)

to disable -> which disables the

2747 ↗

(On Diff #145514)

functions -> function

2748 ↗

(On Diff #145514)

disabling stack -> disabling the stack

2749 ↗

(On Diff #145514)

options -> option

Backticks around the compiler flag.

2751 ↗

(On Diff #145514)

disables stack -> disables the stack

Please put backticks around foo and bar so they highlight as code.

2752 ↗

(On Diff #145514)

with stack -> with the stack
with -fstack-protector -> with the -fstack-protector

Backticks around the compiler flag.

2757 ↗

(On Diff #145514)

C requires parameters to be named; alternatively, you can use the C++ spelling of the attribute.

2759 ↗

(On Diff #145514)

with stack -> with the stack

test/Sema/no_stack_protector.c

4 ↗

(On Diff #145514)

Can you also add a test to ensure the attribute accepts no arguments?

[manojgupta]

Updated test case for error msg with arguments.
Updated the documentation.

[aaron.ballman]

LGTM!

[manojgupta]

Thanks!

include/clang/Basic/Attr.td

 }
 
 def NotTailCalled : InheritableAttr {
   let Spellings = [Clang<"not_tail_called">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NotTailCalledDocs];
 }
 
+def NoStackProtector : InheritableAttr {
+  let Spellings = [GCC<"no_stack_protector">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def NoThrow : InheritableAttr {
   let Spellings = [GCC<"nothrow">, Declspec<"nothrow">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoThrowDocs];
 }
 
 def NvWeak : IgnoredAttr {
   // No Declspec spelling of this attribute; the CUDA headers use

lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   llvm::AttrBuilder B;
 
   if (CodeGenOpts.UnwindTables)
     B.addAttribute(llvm::Attribute::UWTable);
 
   if (!hasUnwindExceptions(LangOpts))
     B.addAttribute(llvm::Attribute::NoUnwind);
 
+  if (!D || !D->hasAttr<NoStackProtectorAttr>()) {
     if (LangOpts.getStackProtector() == LangOptions::SSPOn)
       B.addAttribute(llvm::Attribute::StackProtect);
     else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
       B.addAttribute(llvm::Attribute::StackProtectStrong);
     else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
       B.addAttribute(llvm::Attribute::StackProtectReq);
+  }
 
   if (!D) {
     // If we don't have a declaration to control inlining, the function isn't
     // explicitly marked as alwaysinline for semantic reasons, and inlining is
     // disabled, mark the function as noinline.
     if (!F->hasFnAttribute(llvm::Attribute::AlwaysInline) &&
         CodeGenOpts.getInlining() == CodeGenOptions::OnlyAlwaysInlining)
       B.addAttribute(llvm::Attribute::NoInline);

lib/Sema/SemaDeclAttr.cpp

case AttributeList::AT_Convergent:
     handleSimpleAttribute<ConvergentAttr>(S, D, AL);
     break;
   case AttributeList::AT_NoInline:
     handleSimpleAttribute<NoInlineAttr>(S, D, AL);
     break;
   case AttributeList::AT_NoInstrumentFunction: // Interacts with -pg.
     handleSimpleAttribute<NoInstrumentFunctionAttr>(S, D, AL);
     break;
+  case AttributeList::AT_NoStackProtector:
+    // Interacts with -fstack-protector options.
+    handleSimpleAttribute<NoStackProtectorAttr>(S, D, AL);
+    break;
   case AttributeList::AT_StdCall:
   case AttributeList::AT_CDecl:
   case AttributeList::AT_FastCall:
   case AttributeList::AT_ThisCall:
   case AttributeList::AT_Pascal:
   case AttributeList::AT_RegCall:
   case AttributeList::AT_SwiftCall:
   case AttributeList::AT_VectorCall:

test/CodeGen/stack-protector.c

 
 // DEF: define {{.*}}void @test1(i8* %msg) #[[A:.*]] {
 void test1(const char *msg) {
   char a[strlen(msg) + 1];
   strcpy(a, msg);
   printf("%s\n", a);
 }
 
+// DEF: define {{.*}}void @test2(i8* %msg) #[[B:.*]] {
+__attribute__((no_stack_protector))
+void test2(const char *msg) {
+  char a[strlen(msg) + 1];
+  strcpy(a, msg);
+  printf("%s\n", a);
+}
+
 // NOSSP-NOT: attributes #[[A]] = {{.*}} ssp
 // SSP: attributes #[[A]] = {{.*}} ssp{{ }}
 // SSPSTRONG: attributes #[[A]] = {{.*}} sspstrong
 // SSPREQ: attributes #[[A]] = {{.*}} sspreq
 
 // SAFESTACK-NOSSP: attributes #[[A]] = {{.*}} safestack
 // SAFESTACK-NOSSP-NOT: ssp
 
 // SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp{{ }}
 // SAFESTACK-SSPSTRONG: attributes #[[A]] = {{.*}} safestack sspstrong
 // SAFESTACK-SSPREQ: attributes #[[A]] = {{.*}} safestack sspreq
+
+// NOSSP-NOT: attributes #[[B]] = {{.*}} ssp
+// SSP-NOT: attributes #[[B]] = {{.*}} ssp{{ }}
+// SSPSTRONG-NOT: attributes #[[B]] = {{.*}} sspstrong
+// SSPREQ-NOT: attributes #[[B]] = {{.*}} sspreq
+
+// SAFESTACK-SSP: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSP-NOT: attributes #[[B]] = {{.*}} safestack ssp{{ }}
+// SAFESTACK-SSPSTRONG: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSPSTRONG-NOT: attributes #[[B]] = {{.*}} safestack sspstrong
+// SAFESTACK-SSPREQ: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSPREQ-NOT: attributes #[[B]] = {{.*}} safestack sspreq