aaron.ballman added a comment.

Can you please add some Sema tests that verify the attribute only appertains to 
functions, accepts the right number of arguments, etc? Also, some tests showing 
how this attribute works for things like redelcarations and virtual functions 
would help. e.g.,

  void f();
  __declspec(guard(nocf)) void f() {} // Is this fine?
  
  __declspec(guard(nocf)) void g();
  void g() {} // How about this?
  
  struct S {
    __declspec(guard(nocf)) virtual void f();
  };
  
  struct T : S {
    void f() override; // Should this be guard(nocf) as well?
  };



================
Comment at: clang/include/clang/Basic/AttrDocs.td:4568
+where the programmer has manually inserted "CFG-equivalent" protection. The 
+programmer knows that they are calling through some read only function table 
+whose address is obtained through read only memory references and for which 
the 
----------------
read only -> read-only


================
Comment at: clang/include/clang/Basic/AttrDocs.td:4569
+programmer knows that they are calling through some read only function table 
+whose address is obtained through read only memory references and for which 
the 
+index is masked to the function table limit. This approach may also be applied 
----------------
read only -> read-only


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6635
+  if (!AL.isArgIdent(0)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+        << AL << 0 << AANT_ArgumentIdentifier;
----------------
I think this should use `err_attribute_argument_type` instead, since there's 
only one argument.


================
Comment at: clang/test/CodeGen/guard_nocf.c:15
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: clang/test/CodeGenCXX/guard_nocf.cpp:15
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file

----------------
Please add a newline to the end of the file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72167/new/

https://reviews.llvm.org/D72167



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to