[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:3593
+def EnforceTCBLeaf : InheritableAttr {
+  let Spellings = [Clang<"enforce_tcb_leaf">];
+  let Subjects = SubjectList<[Function]>;
----------------
aaron.ballman wrote:
> Are these two attributes mutually exclusive, or would it make sense for a function to use both of these at the same time?
Indeed! They should be mutually exclusive for the same TCB identifier but the same function can still be a non-leaf member of one TCB and a leaf member of another TCB.

Added an error and a test.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11087
+// TCB warnings
+def err_tcb_conflicting_attributes : Error<
+  "attributes '%0(\"%2\")' and '%1(\"%2\")' are mutually exclusive">;
----------------
Do i understand correctly that while "unknown attribute" is a warning ("must be an attribute for some other compiler, let's ignore"), misuse of a known attribute is typically an error ("ok, whatever you meant here, i have an opinion about what this really means and i don't like it")?


================
Comment at: clang/test/Sema/attr-enforce-tcb-errors.cpp:23
+void both_tcb_and_tcb_leaf_on_separate_redeclarations();
+__attribute__((enforce_tcb_leaf("x"))) // FIXME: This should be an error.
+void both_tcb_and_tcb_leaf_on_separate_redeclarations() {}
----------------
Unfortunately the new facility doesn't catch this case because in `handleEnforceTCBAttr()` the function doesn't yet know that it's a redeclaration. I think this case is more important to catch than the straightforward case (because it's very easy to make this mistake), so i'll try to find a better place to emit the error. Is it ok if this goes into a follow-up patch?


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

https://reviews.llvm.org/D91898