[attributes] Generalize attribute 'enforce_tcb' to Objective-C methods.

Calling an ObjC method from a C function marked with the 'enforce_tcb'
attribute did not produce a warning. Now it does, and on top of that
Objective-C methods can participate in TCBs themselves.

Differential Revision: https://reviews.llvm.org/D122343

Author: pdherbemont

clang/include/clang/Basic/Attr.td

@@ -3915,15 +3915,15 @@ def Builtin : InheritableAttr {
 
 def EnforceTCB : InheritableAttr {
   let Spellings = [Clang<"enforce_tcb">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Args = [StringArgument<"TCBName">];
   let Documentation = [EnforceTCBDocs];
   bit InheritEvenIfAlreadyPresent = 1;
 }
 
 def EnforceTCBLeaf : InheritableAttr {
   let Spellings = [Clang<"enforce_tcb_leaf">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Args = [StringArgument<"TCBName">];
   let Documentation = [EnforceTCBLeafDocs];
   bit InheritEvenIfAlreadyPresent = 1;

clang/include/clang/Sema/Sema.h

@@ -12988,7 +12988,8 @@ class Sema final {
   /// attempts to add itself into the container
   void CheckObjCCircularContainer(ObjCMessageExpr *Message);
 
-  void CheckTCBEnforcement(const CallExpr *TheCall, const FunctionDecl *Callee);
+  void CheckTCBEnforcement(const SourceLocation CallExprLoc,
+                           const NamedDecl *Callee);
 
   void AnalyzeDeleteExprMismatch(const CXXDeleteExpr *DE);
   void AnalyzeDeleteExprMismatch(FieldDecl *Field, SourceLocation DeleteLoc,

clang/lib/Sema/SemaChecking.cpp

@@ -5497,7 +5497,9 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
   if (!FnInfo)
     return false;
 
-  CheckTCBEnforcement(TheCall, FDecl);
+  // Enforce TCB except for builtin calls, which are always allowed.
+  if (FDecl->getBuiltinID() == 0)
+    CheckTCBEnforcement(TheCall->getExprLoc(), FDecl);
 
   CheckAbsoluteValueFunction(TheCall, FDecl);
   CheckMaxUnsignedZero(TheCall, FDecl);
@@ -5537,6 +5539,8 @@ bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac,
             /*IsMemberFunction=*/false, lbrac, Method->getSourceRange(),
             CallType);
 
+  CheckTCBEnforcement(lbrac, Method);
+
   return false;
 }
 
@@ -17359,13 +17363,11 @@ ExprResult Sema::SemaBuiltinMatrixColumnMajorStore(CallExpr *TheCall,
 /// CheckTCBEnforcement - Enforces that every function in a named TCB only
 /// directly calls other functions in the same TCB as marked by the enforce_tcb
 /// and enforce_tcb_leaf attributes.
-void Sema::CheckTCBEnforcement(const CallExpr *TheCall,
-                               const FunctionDecl *Callee) {
-  const FunctionDecl *Caller = getCurFunctionDecl();
+void Sema::CheckTCBEnforcement(const SourceLocation CallExprLoc,
+                               const NamedDecl *Callee) {
+  const NamedDecl *Caller = getCurFunctionOrMethodDecl();
 
-  // Calls to builtins are not enforced.
-  if (!Caller || !Caller->hasAttr<EnforceTCBAttr>() ||
-      Callee->getBuiltinID() != 0)
+  if (!Caller || !Caller->hasAttr<EnforceTCBAttr>())
     return;
 
   // Search through the enforce_tcb and enforce_tcb_leaf attributes to find
@@ -17383,9 +17385,8 @@ void Sema::CheckTCBEnforcement(const CallExpr *TheCall,
       [&](const auto *A) {
         StringRef CallerTCB = A->getTCBName();
         if (CalleeTCBs.count(CallerTCB) == 0) {
-          this->Diag(TheCall->getExprLoc(),
-                     diag::warn_tcb_enforcement_violation) << Callee
-                                                           << CallerTCB;
+          this->Diag(CallExprLoc, diag::warn_tcb_enforcement_violation)
+              << Callee << CallerTCB;
         }
       });
 }

clang/test/Misc/pragma-attribute-supported-attributes-list.test

@@ -61,8 +61,8 @@
 // CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
-// CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
-// CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function)
+// CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnumExtensibility (SubjectMatchRule_enum)
 // CHECK-NEXT: Error (SubjectMatchRule_function)
 // CHECK-NEXT: ExcludeFromExplicitInstantiation (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)

clang/test/Sema/attr-enforce-tcb-errors.m

@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo();
+
+__attribute__((objc_root_class))
+@interface AClass
+- (void)bothTCBAndTCBLeafOnSeparateRedeclarations __attribute__((enforce_tcb("x"))); // expected-note{{conflicting attribute is here}}
+
+- (void)bothTCBAndTCBLeafOnSeparateRedeclarationsOppositeOrder __attribute__((enforce_tcb_leaf("x"))); // expected-note{{conflicting attribute is here}}
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiersOnSeparateRedeclarations __attribute__((enforce_tcb("x")));
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiersOnSeparateRedeclarationsOppositeOrder __attribute__((enforce_tcb_leaf("x")));
+
+- (void)onInterfaceOnly __attribute__((enforce_tcb("test")));
+@end
+
+@interface AClass (NoImplementation)
+- (void)noArguments __attribute__((enforce_tcb)); // expected-error{{'enforce_tcb' attribute takes one argument}}
+
+- (void)tooManyArguments __attribute__((enforce_tcb("test", 12))); // expected-error{{'enforce_tcb' attribute takes one argument}}
+
+- (void)wrongArgumentType __attribute__((enforce_tcb(12))); // expected-error{{'enforce_tcb' attribute requires a string}}
+
+- (void)noArgumentsLeaf __attribute__((enforce_tcb_leaf)); // expected-error{{'enforce_tcb_leaf' attribute takes one argument}}
+
+- (void)tooManyArgumentsLeaf __attribute__((enforce_tcb_leaf("test", 12))); // expected-error{{'enforce_tcb_leaf' attribute takes one argument}}
+
+- (void)wrongArgumentTypeLeaf __attribute__((enforce_tcb_leaf(12))); // expected-error{{'enforce_tcb_leaf' attribute requires a string}}
+@end
+
+@implementation AClass
+- (void)onInterfaceOnly {
+  foo(); // expected-warning{{calling 'foo' is a violation of trusted computing base 'test'}}
+}
+
+- (void)bothTCBAndTCBLeaf
+    __attribute__((enforce_tcb("x")))
+    __attribute__((enforce_tcb_leaf("x"))) // expected-error{{attributes 'enforce_tcb_leaf("x")' and 'enforce_tcb("x")' are mutually exclusive}}
+{
+  foo(); // no-warning
+}
+
+- (void)bothTCBAndTCBLeafOnSeparateRedeclarations
+    __attribute__((enforce_tcb_leaf("x"))) // expected-error{{attributes 'enforce_tcb_leaf("x")' and 'enforce_tcb("x")' are mutually exclusive}}
+{
+  // Error recovery: no need to emit a warning when we didn't
+  // figure out our attributes to begin with.
+  foo(); // no-warning
+}
+
+- (void)bothTCBAndTCBLeafOppositeOrder
+    __attribute__((enforce_tcb_leaf("x")))
+    __attribute__((enforce_tcb("x"))) // expected-error{{attributes 'enforce_tcb("x")' and 'enforce_tcb_leaf("x")' are mutually exclusive}}
+{
+  foo(); // no-warning
+}
+
+- (void)bothTCBAndTCBLeafOnSeparateRedeclarationsOppositeOrder
+    __attribute__((enforce_tcb("x"))) // expected-error{{attributes 'enforce_tcb("x")' and 'enforce_tcb_leaf("x")' are mutually exclusive}}
+{
+  foo(); // no-warning
+}
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiers
+    __attribute__((enforce_tcb("x")))
+    __attribute__((enforce_tcb_leaf("y"))) // no-error
+{
+  foo(); // expected-warning{{calling 'foo' is a violation of trusted computing base 'x'}}
+}
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiersOppositeOrder
+    __attribute__((enforce_tcb_leaf("x")))
+    __attribute__((enforce_tcb("y"))) // no-error
+{
+  foo(); // expected-warning{{calling 'foo' is a violation of trusted computing base 'y'}}
+}
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiersOnSeparateRedeclarations
+    __attribute__((enforce_tcb_leaf("y"))) // no-error
+{
+  foo(); // expected-warning{{calling 'foo' is a violation of trusted computing base 'x'}}
+}
+
+- (void)bothTCBAndTCBLeafButDifferentIdentifiersOnSeparateRedeclarationsOppositeOrder
+    __attribute__((enforce_tcb("y"))) {
+  foo(); // expected-warning{{calling 'foo' is a violation of trusted computing base 'y'}}
+}
+
+- (void)errorRecoveryOverIndividualTCBs
+    __attribute__((enforce_tcb("y")))
+    __attribute__((enforce_tcb("x")))
+    __attribute__((enforce_tcb_leaf("x"))) // expected-error{{attributes 'enforce_tcb_leaf("x")' and 'enforce_tcb("x")' are mutually exclusive}}
+{
+  // FIXME: Ideally this should warn. The conflict between attributes
+  // for TCB "x" shouldn't affect the warning about TCB "y".
+  foo(); // no-warning
+}
+
+@end

clang/test/Sema/attr-enforce-tcb.m

@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define PLACE_IN_TCB(NAME) __attribute__((enforce_tcb(NAME)))
+#define PLACE_IN_TCB_LEAF(NAME) __attribute__((enforce_tcb_leaf(NAME)))
+
+__attribute__((objc_root_class))
+@interface AClass
+@property(readonly) id propertyNotInAnyTCB;
+@end
+
+@implementation AClass
+- (void)inTCBFoo PLACE_IN_TCB("foo") {
+  [self notInAnyTCB]; // expected-warning{{calling 'notInAnyTCB' is a violation of trusted computing base 'foo'}}
+}
+- (void)inTCBFooAsLeaf PLACE_IN_TCB_LEAF("foo") {
+  [self notInAnyTCB]; // no-warning
+}
+- (void)notInAnyTCB {
+}
++ (void)classMethodNotInAnyTCB {
+}
++ (void)classMethodInTCBFoo PLACE_IN_TCB("foo") {
+  [self inTCBFoo];       // no-warning
+  [self inTCBFooAsLeaf]; // no-warning
+  [self notInAnyTCB];    // expected-warning{{calling 'notInAnyTCB' is a violation of trusted computing base 'foo'}}
+}
+@end
+
+PLACE_IN_TCB("foo")
+void call_objc_method(AClass *object) {
+  [object inTCBFoo];                // no-warning
+  [object inTCBFooAsLeaf];          // no-warning
+  [object notInAnyTCB];             // expected-warning{{calling 'notInAnyTCB' is a violation of trusted computing base 'foo'}}
+  [AClass classMethodNotInAnyTCB];  // expected-warning{{calling 'classMethodNotInAnyTCB' is a violation of trusted computing base 'foo'}}
+  [AClass classMethodInTCBFoo];     // no-warning
+  (void)object.propertyNotInAnyTCB; // expected-warning{{calling 'propertyNotInAnyTCB' is a violation of trusted computing base 'foo'}}
+}