xazax.hun updated this revision to Diff 232681.
xazax.hun marked 7 inline comments as done.
xazax.hun added a comment.

- Make the annotation acceptable on both types and declarations.
- Fix some TODOs.
- Teach TypePrinter to print the annotations.
- Address review comments.


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

https://reviews.llvm.org/D70469

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-handles.cpp

Index: clang/test/Sema/attr-handles.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/attr-handles.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+// Decl annotations.
+void f(int *a __attribute__((acquire_handle)));
+void (*fp)(int handle [[clang::use_handle]]);
+auto lambda = [](int handle [[clang::use_handle]]){};
+void g(int a __attribute__((acquire_handle))); // expected-error {{attribute only applies to output parameters}}
+void h(int *a __attribute__((acquire_handle(1)))); // expected-error {{'acquire_handle' attribute takes no arguments}}
+__attribute__((release_handle)) int i(); // expected-warning {{'release_handle' attribute only applies to parameters}}
+__attribute__((use_handle)) int j(); // expected-warning {{'use_handle' attribute only applies to parameters}}
+int a __attribute__((acquire_handle)); // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}}
+
+// Type annotations.
+void ft(int __attribute__((acquire_handle)) * a);
+void (*fpt)(int __attribute__((use_handle)) handle);
+auto lambdat = [](int __attribute__((use_handle)) handle) ->
+    int __attribute__((acquire_handle)) { return 0; };
+void gt(int __attribute__((acquire_handle)) a); // expected-error {{attribute only applies to output parameters}}
+void ht(int __attribute__((acquire_handle(1))) *a); // expected-error {{'acquire_handle' attribute takes no arguments}}
+int it() __attribute__((release_handle)); // expected-warning {{'release_handle' only applies to non-function types; type here is 'int ()'}}
+int jt() __attribute__((use_handle)); // expected-warning {{'use_handle' only applies to non-function types; type here is 'int ()'}}
+int __attribute__((acquire_handle)) at; // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}}
\ No newline at end of file
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===================================================================
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_hasType_functionType, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
@@ -128,6 +129,7 @@
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: Restrict (SubjectMatchRule_function)
@@ -145,6 +147,7 @@
 // CHECK-NEXT: Target (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
+// CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7418,6 +7418,21 @@
          attrKind == ParsedAttr::AT_OpenCLGenericAddressSpace;
 }
 
+template <typename AttrTy>
+static void handleHandleAttr(TypeProcessingState &State, QualType &CurType,
+                             ParsedAttr &Attr) {
+  if (CurType->isFunctionType()) {
+    State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type)
+        << Attr.getAttrName()->getName() << 3 << CurType;
+    Attr.setInvalid();
+    return;
+  }
+  ASTContext &Ctx = State.getSema().Context;
+  CurType = State.getAttributedType(createSimpleAttr<AttrTy>(Ctx, Attr),
+                                    CurType, CurType);
+  Attr.setUsedAsTypeAttr();
+}
+
 static void processTypeAttrs(TypeProcessingState &state, QualType &type,
                              TypeAttrLocation TAL,
                              ParsedAttributesView &attrs) {
@@ -7614,6 +7629,15 @@
       else if (!handleFunctionTypeAttr(state, attr, type))
         distributeFunctionTypeAttr(state, attr, type);
       break;
+    case ParsedAttr::AT_UseHandle:
+      handleHandleAttr<UseHandleAttr>(state, type, attr);
+      break;
+    case ParsedAttr::AT_AcquireHandle:
+      handleHandleAttr<AcquireHandleAttr>(state, type, attr);
+      break;
+    case ParsedAttr::AT_ReleaseHandle:
+      handleHandleAttr<ReleaseHandleAttr>(state, type, attr);
+      break;
     }
 
     // Handle attributes that are defined in a macro. We do not want this to be
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6579,6 +6579,18 @@
   handleSimpleAttribute<MSAllocatorAttr>(S, D, AL);
 }
 
+static void handeAcquireHandleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // Warn if the parameter is definitely not an output parameter.
+  if (const auto *PVD = dyn_cast<ParmVarDecl>(D)) {
+    if (PVD->getType()->isIntegerType()) {
+      S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)
+          << AL.getRange();
+      return;
+    }
+  }
+  handleSimpleAttribute<AcquireHandleAttr>(S, D, AL);
+}
+
 //===----------------------------------------------------------------------===//
 // Top Level Sema Entry Points
 //===----------------------------------------------------------------------===//
@@ -7353,6 +7365,16 @@
   case ParsedAttr::AT_ArmMveAlias:
     handleArmMveAliasAttr(S, D, AL);
     break;
+
+  case ParsedAttr::AT_AcquireHandle:
+    handeAcquireHandleAttr(S, D, AL);
+    break;
+  case ParsedAttr::AT_ReleaseHandle:
+    handleSimpleAttribute<ReleaseHandleAttr>(S, D, AL);
+    break;
+  case ParsedAttr::AT_UseHandle:
+    handleSimpleAttribute<UseHandleAttr>(S, D, AL);
+    break;
   }
 }
 
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1555,6 +1555,15 @@
   case attr::NoDeref:
     OS << "noderef";
     break;
+  case attr::UseHandle:
+    OS << "use_handle";
+    break;
+  case attr::AcquireHandle:
+    OS << "acquire_handle";
+    break;
+  case attr::ReleaseHandle:
+    OS << "release_handle";
+    break;
   }
   OS << "))";
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3060,7 +3060,7 @@
 def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Text>;
 def warn_type_attribute_wrong_type : Warning<
   "'%0' only applies to %select{function|pointer|"
-  "Objective-C object or block pointer}1 types; type here is %2">,
+  "Objective-C object or block pointer|non-function}1 types; type here is %2">,
   InGroup<IgnoredAttributes>;
 def warn_incomplete_encoded_type : Warning<
   "encoding of %0 type is incomplete because %1 component has unknown encoding">,
@@ -3121,6 +3121,8 @@
 def err_cconv_incomplete_param_type : Error<
   "parameter %0 must have a complete type to use function %1 with the %2 "
   "calling convention">;
+def err_attribute_output_parameter : Error<
+  "attribute only applies to output parameters">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup<IgnoredAttributes>;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4643,3 +4643,72 @@
   }
   }];
 }
+
+def HandleDocs : DocumentationCategory<"Handle Attributes"> {
+  let Content = [{
+Handles are a way to identify resources like files, sockets, and processes.
+They are more opaque than pointers and widely used in system programming. They
+have similar risks such as never releasing a resource associated with a handle,
+attempting to use a handle that was already released, or trying to release a
+handle twice. Using the annotations below it is possible to make the ownership
+of the handles clear: whose responsibility is to release them. They can also
+aid static analysis tools to find bugs.
+  }];
+}
+
+def AcquireHandleDocs : Documentation {
+  let Category = HandleDocs;
+  let Content = [{
+If this annotation is on a function or a function type it is assumed to return
+a new handle. In case this annotation is on an output parameter or on the handle
+type, the function is assumed to fill the corresponding argument with a new
+handle.
+
+.. code-block:: c++
+
+  // Output arguments from Zircon.
+  zx_status_t zx_socket_create(uint32_t options,
+                               zx_handle_t __attribute__((acquire_handle)) * out0,
+                               zx_handle_t* out1 [[clang::acquire_handle]]);
+
+
+  // Returned handle.
+  [[clang::acquire_handle]] int open(const char *path, int oflag, ... );
+  int open(const char *path, int oflag, ... ) __attribute__((acquire_handle));
+  }];
+}
+
+def UseHandleDocs : Documentation {
+  let Category = HandleDocs;
+  let Content = [{
+A function taking a handle by value might close the handle. If a function
+parameter or the parameter's handle type is annotated with `use_handle` it is
+assumed to not to change the state of the handle. It is also assumed to require
+an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_port_wait(zx_handle_t handle [[clang::use_handle]],
+                           zx_time_t deadline,
+                           zx_port_packet_t* packet);
+  // or
+  zx_status_t zx_port_wait(zx_handle_t __attribute__((use_handle)) handle,
+                           zx_time_t deadline,
+                           zx_port_packet_t* packet);
+  }];
+}
+
+def ReleaseHandleDocs : Documentation {
+  let Category = HandleDocs;
+  let Content = [{
+If a function parameter or the parameter's handle type is annotated with
+`release_handle` it is assumed to close the handle. It is also assumed to
+require an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_handle_close(zx_handle_t handle [[clang::release_handle]]);
+  // or
+  zx_status_t zx_handle_close(zx_handle_t __attribute__((release_handle)) handle);
+  }];
+}
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3473,3 +3473,21 @@
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoBuiltinDocs];
 }
+
+def AcquireHandle : DeclOrTypeAttr {
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[FunctionLike, ParmVar]>;
+  let Documentation = [AcquireHandleDocs];
+}
+
+def UseHandle : DeclOrTypeAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [UseHandleDocs];
+}
+
+def ReleaseHandle : DeclOrTypeAttr {
+  let Spellings = [Clang<"release_handle">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [ReleaseHandleDocs];
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to