-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[Clang][attr] Add 'cfi_salt' attribute #141846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
bfdf16c
04d2c0b
94d10c5
bbce666
e36bc7c
b0c8a2e
9e164e5
eef5f43
46d0edd
f4a54d9
e9aaaf5
0b13d3a
cec156a
44950cc
514523b
f74c5ea
527297f
2df94a9
66cb83b
516aa05
1b1d3ab
6d00a7e
2e9022d
7f2de1c
44c179d
029ee8e
06e43bd
132995f
cd975ea
abe3bde
c2dba02
95698c2
09a446a
ae91164
9bd2c95
9700cac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3843,6 +3843,13 @@ def CFICanonicalJumpTable : InheritableAttr { | |
let SimpleHandler = 1; | ||
} | ||
|
||
def CFISalt : DeclOrTypeAttr { | ||
let Spellings = [Clang<"cfi_salt">]; | ||
let Args = [StringArgument<"Salt">]; | ||
let Subjects = SubjectList<[Function, Field, Var, TypedefName], ErrorDiag>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for missing this earlier, but should this be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are tests applying to a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's specifically about the placement (it's complicated). If this is stuff you already know, feel free to ignore (not trying to mansplain, I just have no idea how much folks know about these kinds of details). For attributes spelled with
So based on this previously being a
but this behavior is specific to With your latest changes to use
(Again, this is specific to the behavior with the CC @erichkeane as attributes code owner in case he disagrees with anything I'm saying above or has better explanations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That limits some uses. @kees, is it okay to abandon these constructs?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect those cases to work, at least with the https://godbolt.org/z/fY7jMGTsj is a similar example showing what I mean. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added them back. |
||
let Documentation = [CFISaltDocs]; | ||
} | ||
|
||
// C/C++ Thread safety attributes (e.g. for deadlock, data race checking) | ||
// Not all of these attributes will be given a [[]] spelling. The attributes | ||
// which require access to function parameter names cannot use the [[]] spelling | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,6 +164,7 @@ static void diagnoseBadTypeAttribute(Sema &S, const ParsedAttr &attr, | |
case ParsedAttr::AT_ArmOut: \ | ||
case ParsedAttr::AT_ArmInOut: \ | ||
case ParsedAttr::AT_ArmAgnostic: \ | ||
case ParsedAttr::AT_CFISalt: \ | ||
case ParsedAttr::AT_AnyX86NoCallerSavedRegisters: \ | ||
case ParsedAttr::AT_AnyX86NoCfCheck: \ | ||
CALLING_CONV_ATTRS_CASELIST | ||
|
@@ -7943,6 +7944,21 @@ static bool handleFunctionTypeAttr(TypeProcessingState &state, ParsedAttr &attr, | |
return true; | ||
} | ||
|
||
if (attr.getKind() == ParsedAttr::AT_CFISalt) { | ||
StringRef Argument; | ||
if (!S.checkStringLiteralArgumentAttr(attr, 0, Argument)) | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah...I'm not sure why I missed that. Fixed. |
||
|
||
const auto *FnTy = unwrapped.get()->getAs<FunctionProtoType>(); | ||
FunctionProtoType::ExtProtoInfo EPI = FnTy->getExtProtoInfo(); | ||
EPI.ExtraAttributeInfo.CFISalt = Argument; | ||
|
||
QualType newtype = S.Context.getFunctionType(FnTy->getReturnType(), | ||
FnTy->getParamTypes(), EPI); | ||
type = unwrapped.wrap(S, newtype->getAs<FunctionType>()); | ||
return true; | ||
} | ||
|
||
if (attr.getKind() == ParsedAttr::AT_ArmStreaming || | ||
attr.getKind() == ParsedAttr::AT_ArmStreamingCompatible || | ||
attr.getKind() == ParsedAttr::AT_ArmPreserves || | ||
|
Uh oh!
There was an error while loading. Please reload this page.