[clang] Thread Safety Analysis: Support warning on taking address of guarded variables (PR #123063)

via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 19 12:27:46 PST 2025


aoates wrote:

> > I'm very excited about this, as I have wanted it for many years for my C codebase, and TSA is not super useful in C without this!
> 
> This PR is being superseded by #127396 (implementation changed completely) - we agreed to go with the more conservative approach, and at the same time also properly handle pt_guarded_by pointers. Wthread-safety-pointer is meant to be to pointer passing what Wthread-safety-reference is to C++ reference passing.
> 
> I'm leaving this PR up to keep the history.

Ah great, I will follow along there. I might patch in the early version of this and give it a spin this week (I don't want to wait for it be merged :D)
> 
> > One thought --- you could consider an attribute that could be put on pointer arguments to functions that says "yes, I dereference this and read or write it". In a codebase that otherwise would have many false positives, you could annotate at least core data structures without having to turn it on for all address-of operations.
> > E.g `void hashtable_insert(htbl_t* WRITES_POINTER table, ...)`
> > In the C codebase I desperately want this for, an annotation like that sprinkled in a couple key places would get you 80% of the benefit with much lower risk of false positives.
> 
> Perhaps if there are false positives this can be added. But initially I'd like to avoid it if at all possible. A similar precedent at least for passing references to functions exists with `Wthread-safety-reference` (for C++ references), so in a first iteration we have narrowed it down to effectively mirror Wthread-safety-reference but for pointers.
> 
> FWIW, something similar to explicit marking can be achieved with something like this (but yes, it's ugly):
> 
> ```
> void __hashtable_insert(htbl_t* table, ...);
> #define hashtable_insert(table, ...) ({ (void)(*(table)); __hashtable_insert(table, ...); })

Yup, this is what I currently am playing with, but it's ugly (and requires double-evaluating the macro argument which screws up side effects).

> ```
> 
> Although we might need this patch too, to handle things like `*&var`: [a70f021](https://github.com/llvm/llvm-project/commit/a70f021becb2888d6c2a63b2d1e619393a996058)



https://github.com/llvm/llvm-project/pull/123063


More information about the cfe-commits mailing list