Page MenuHomePhabricator

[Clang] Implement function attribute no_stack_protector.

Authored by manojgupta on Apr 30 2018, 10:34 PM.



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.

Diff Detail

rC Clang

Event Timeline

manojgupta created this revision.Apr 30 2018, 10:34 PM

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

aaron.ballman requested changes to this revision.May 1 2018, 6:35 PM

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


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")?


No new undocumented attributes, please.

This revision now requires changes to proceed.May 1 2018, 6:35 PM

Added docs, Sema test case for the attribute.

manojgupta added inline comments.May 7 2018, 12:39 PM

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.

__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 edited reviewers, added: probinson; removed: void.May 7 2018, 1:38 PM
aaron.ballman added a subscriber: probinson.

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?


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 added inline comments.May 8 2018, 5:18 PM

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 added inline comments.May 9 2018, 5:26 AM

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


to disable -> which disables the


functions -> function


disabling stack -> disabling the stack


options -> option

Backticks around the compiler flag.


disables stack -> disables the stack

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


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

Backticks around the compiler flag.


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


with stack -> with the stack


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

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

This revision is now accepted and ready to land.May 9 2018, 1:43 PM
This revision was automatically updated to reflect the committed changes.