Unverified Commit d1e84bb9 authored by Marco Elver's avatar Marco Elver Committed by GitHub
Browse files

Thread Safety Analysis: Support attributes on function pointers (#191187) 

Allow acquire_capability, release_capability, requires_capability,
try_acquire_capability, assert_capability, and locks_excluded attributes
(incl. their shared variants) on function pointer variables and struct
fields. Calls through annotated function pointers are checked the same
way as direct function calls.

The attributes are placed on variable/field declarations, not on the
function pointer type itself. This is a deliberate trade-off: making
these "attributes" part of the type system would require diagnosing
mismatched assignments, which would be a significant type-system
extension with limited practical benefit, which would likely require
promoting the TSA vocabulary to full type-qualifiers. Instead, the
analysis trusts the annotations on the variable at the call site, and
sticks with the attribute-based semantics. This matches the existing
philosophy where the analysis tries to avoid false positives where
possible and attribute mismatches on direct functions are likewise not
hard errors or warnings (yet).

The primary motivation is to avoid false positives in large C codebases,
such as the Linux kernel [1], which tend to use structs containing
function pointers to emulate subtype polymorphism and dynamic dispatch.

[1] https://lore.kernel.org/all/20260409064221.GA8378@lst.de/
parent 37f16ad2

clang/docs/ReleaseNotes.rst

+8 −0
Original line number Diff line number Diff line
@@ -324,6 +324,14 @@ Attribute Changes in Clang 
  sound because any writer must hold all capabilities, so holding any one

  prevents concurrent writes.



- :doc:`ThreadSafetyAnalysis` attributes like ``acquire_capability``,

  ``release_capability``, ``requires_capability``, ``locks_excluded``,

  ``try_acquire_capability``, and ``assert_capability`` can now be applied to

  function pointer variables and fields.  The analysis checks calls through

  annotated function pointers the same way it checks direct function calls.

  Only plain function pointers are supported; pointers-to-member functions,

  blocks, or wrappers (e.g. ``std::function``) are not yet supported.



- The ``[[clang::unsafe_buffer_usage]]`` attribute is now supported in API

  notes. For example:

clang/docs/ThreadSafetyAnalysis.rst

+36 −0
Original line number Diff line number Diff line
@@ -544,6 +544,42 @@ GUARDED_VAR and PT_GUARDED_VAR 
Use of these attributes has been deprecated.





Function Pointers

-----------------



Thread safety attributes may also be applied to function pointer variables and

fields.  The attributes describe the locking behavior of calling through that

pointer, and the analysis will check calls through the pointer accordingly.



.. code-block:: c++



  Mutex mu;

  int x GUARDED_BY(mu);



  void (*lock_fn)(void)   ACQUIRE(mu);

  void (*unlock_fn)(void) RELEASE(mu);



  struct Ops {

    void (*read)(void) REQUIRES(mu);

  };



  void test(Ops *ops) {

    lock_fn();

    x = 1;

    ops->read();

    unlock_fn();

  }



Note that the attributes are on the *variable* (or field), not on the function

pointer type.  Assigning a function with different (or no) attributes to an

annotated function pointer variable is not diagnosed.  The analysis trusts the

annotations on the variable at the call site.



This support is limited to plain function pointers.  Pointers-to-member

functions, blocks, and wrapper types such as ``std::function`` are not

supported yet.





Warning flags

-------------

clang/include/clang/Basic/Attr.td

+16 −6
Original line number Diff line number Diff line
@@ -747,6 +747,9 @@ class Attr { 
  bit PragmaAttributeSupport;

  // Set to true if this attribute accepts parameter pack expansion expressions.

  bit AcceptsExprPack = 0;

  // Set to true if this attribute's argument list is parsed inside a function

  // prototype scope, so it can reference the function's parameters.

  bit ParseArgsInFunctionScope = 0;

  // To support multiple enum parameters to an attribute without breaking

  // our existing general parsing we need to have a separate flag that

  // opts an attribute into strict parsing of attribute parameters
@@ -1910,6 +1913,7 @@ def EnableIf : InheritableAttr { 
  let Subjects = SubjectList<[Function]>;

  let Args = [ExprArgument<"Cond">, StringArgument<"Message">];

  let TemplateDependent = 1;

  let ParseArgsInFunctionScope = 1;

  let Documentation = [EnableIfDocs];

}
@@ -4096,11 +4100,12 @@ def AssertCapability : InheritableAttr { 
                   Clang<"assert_shared_capability", 0>,

                   GNU<"assert_exclusive_lock">,

                   GNU<"assert_shared_lock">];

  let Subjects = SubjectList<[Function]>;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let LateParsed = LateAttrParseStandard;

  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let ParseArgsInFunctionScope = 1;

  let Args = [VariadicExprArgument<"Args">];

  let AcceptsExprPack = 1;

  let Accessors = [Accessor<"isShared",
@@ -4114,11 +4119,12 @@ def AcquireCapability : InheritableAttr { 
                   Clang<"acquire_shared_capability", 0>,

                   GNU<"exclusive_lock_function">,

                   GNU<"shared_lock_function">];

  let Subjects = SubjectList<[Function, ParmVar]>;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let LateParsed = LateAttrParseStandard;

  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let ParseArgsInFunctionScope = 1;

  let Args = [VariadicExprArgument<"Args">];

  let AcceptsExprPack = 1;

  let Accessors = [Accessor<"isShared",
@@ -4132,11 +4138,12 @@ def TryAcquireCapability : InheritableAttr { 
                   Clang<"try_acquire_shared_capability", 0>,

                   GNU<"exclusive_trylock_function">,

                   GNU<"shared_trylock_function">];

  let Subjects = SubjectList<[Function]>;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let LateParsed = LateAttrParseStandard;

  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let ParseArgsInFunctionScope = 1;

  let Args = [ExprArgument<"SuccessValue">, VariadicExprArgument<"Args">];

  let AcceptsExprPack = 1;

  let Accessors = [Accessor<"isShared",
@@ -4150,11 +4157,12 @@ def ReleaseCapability : InheritableAttr { 
                   Clang<"release_shared_capability", 0>,

                   Clang<"release_generic_capability", 0>,

                   Clang<"unlock_function", 0>];

  let Subjects = SubjectList<[Function, ParmVar]>;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let LateParsed = LateAttrParseStandard;

  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let ParseArgsInFunctionScope = 1;

  let Args = [VariadicExprArgument<"Args">];

  let AcceptsExprPack = 1;

  let Accessors = [Accessor<"isShared",
@@ -4176,7 +4184,8 @@ def RequiresCapability : InheritableAttr { 
  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let Subjects = SubjectList<[Function, ParmVar]>;

  let ParseArgsInFunctionScope = 1;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>,

                                         Clang<"shared_locks_required", 0>]>];

  let Documentation = [Undocumented];
@@ -4253,7 +4262,8 @@ def LocksExcluded : InheritableAttr { 
  let TemplateDependent = 1;

  let ParseArgumentsAsUnevaluated = 1;

  let InheritEvenIfAlreadyPresent = 1;

  let Subjects = SubjectList<[Function, ParmVar]>;

  let ParseArgsInFunctionScope = 1;

  let Subjects = SubjectList<[Function, Var, Field]>;

  let Documentation = [Undocumented];

}

clang/include/clang/Basic/DiagnosticSemaKinds.td

+3 −0
Original line number Diff line number Diff line
@@ -4306,6 +4306,9 @@ def warn_thread_attribute_not_on_scoped_lockable_param : Warning< 
def warn_thread_attribute_requires_preceded : Warning<

  "%0 attribute on %1 must be preceded by %2 attribute">,

  InGroup<ThreadSafetyAttributes>, DefaultIgnore;

def warn_thread_attribute_not_on_fun_ptr : Warning<

  "%0 attribute on a %select{variable|field}1 requires the %select{variable|field}1 to be of function pointer type">,

  InGroup<ThreadSafetyAttributes>, DefaultIgnore;

def err_attribute_argument_out_of_bounds_extra_info : Error<

  "%0 attribute parameter %1 is out of bounds: "

  "%plural{0:no parameters to index into|"

clang/include/clang/Sema/Sema.h

+4 −0
Original line number Diff line number Diff line
@@ -14251,6 +14251,10 @@ public: 
  };

  typedef SmallVector<LateInstantiatedAttribute, 1> LateInstantiatedAttrVec;













  /// Recheck instantiated thread-safety attributes that could not be validated













  /// on the dependent pattern declaration.













  bool checkInstantiatedThreadSafetyAttrs(const Decl *D, const Attr *A);



























  void InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,















                        const Decl *Pattern, Decl *Inst,















                        LateInstantiatedAttrVec *LateAttrs = nullptr,