[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/Attr.td:2089
+def AnyX86NoCfCheck : InheritableAttr, TargetSpecificAttr<TargetAnyX86>{
+  let Spellings = [GCC<"nocf_check">];
+  let Documentation = [AnyX86NoCfCheckDocs];
----------------
This attribute doesn't appear to be supported by GCC. Did you mean to use the Clang spelling instead?


================
Comment at: include/clang/Basic/AttrDocs.td:2876
+  let Content = [{
+Jump Oriented Programming attacks rely on tampering addresses used by
+indirect call / jmp, e.g. redirect control-flow to non-programmer
----------------
tampering with addresses


================
Comment at: include/clang/Basic/AttrDocs.td:2878
+indirect call / jmp, e.g. redirect control-flow to non-programmer
+intended bytes in binary.
+X86 Supports Indirect Branch Tracking (IBT) as part of Control-Flow
----------------
bytes in the binary


================
Comment at: lib/Sema/SemaDeclAttr.cpp:1990
 
-bool Sema::CheckNoReturnAttr(const AttributeList &Attrs) {
-  if (!checkAttributeNumArgs(*this, Attrs, 0)) {
-    Attrs.setInvalid();
+static void handleNoCfCheckAttr(Sema &S, Decl *D, const AttributeList &attr) {
+  if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))
----------------
`attr` doesn't follow the proper naming conventions.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:1991-1992
+static void handleNoCfCheckAttr(Sema &S, Decl *D, const AttributeList &attr) {
+  if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))
+    return;
+
----------------
Why do you need to manually check the attribute has no args -- doesn't the common handling in `handleCommonAttributeFeatures()` already cover that?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:1994-2002
+  if (!isFunctionOrMethod(D)) {
+    ValueDecl *VD = dyn_cast<ValueDecl>(D);
+    if (!VD || (!VD->getType()->isFunctionPointerType())) {
+      S.Diag(attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+          << attr.getName() << ExpectedFunctionOrMethod;
+      return;
+    }
----------------
I think all of this can be replaced by setting the proper subject on the attribute (`FunctionLike`) in Attr.td.


================
Comment at: test/Sema/attr-nocf_check.c:7-9
+// Allow function declaration and definition mismatch.
+void __attribute__((nocf_check)) testNoCfCheck();   // no-warning
+void __attribute__((nocf_check)) testNoCfCheck(){}; // no-warning
----------------
How is this mismatched?


Repository:
  rL LLVM

https://reviews.llvm.org/D41880