Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (#110523)

This is helpful when multiple functions operate on the same
capabilities, but we still w

Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (#110523)

This is helpful when multiple functions operate on the same
capabilities, but we still want to use scoped lockable types for
readability and exception safety.
- Introduce support for thread safety annotations on function parameters
marked with the 'scoped_lockable' attribute.
- Add semantic checks for annotated function parameters, ensuring
correct usage.
- Enhance the analysis to recognize and handle parameters annotated for
thread safety, extending the scope of analysis to track these across
function boundries.
- Verify that the underlying mutexes of function arguments match the
expectations set by the annotations.

Limitation: This does not work when the attribute arguments are class
members, because attributes on function parameters are parsed
differently from attributes on functions.

show more ...

[Analysis] Avoid repeated hash lookups (NFC) (#107357)

Thread Safety Analysis: Differentiate between lock sets at real join points and expected/actual sets at function end (#105526)

This fixes false positives related to returning a scoped lockable
obje

Thread Safety Analysis: Differentiate between lock sets at real join points and expected/actual sets at function end (#105526)

This fixes false positives related to returning a scoped lockable
object. At the end of a function, we check managed locks instead of
scoped locks.

At real join points, we skip checking managed locks because we assume
that the scope keeps track of its underlying mutexes and will release
them at its destruction. So, checking for the scopes is sufficient.
However, at the end of a function, we aim at comparing the expected and
the actual lock sets. There, we skip checking scoped locks to prevent to
get duplicate warnings for the same lock.

show more ...

[clang][ThreadSafety] Revert stricter typing on trylock attributes (#97293)

This PR reverts #95290 and the one-liner followup PR #96494.

I received some substantial feedback on #95290, which I pl

[clang][ThreadSafety] Revert stricter typing on trylock attributes (#97293)

This PR reverts #95290 and the one-liner followup PR #96494.

I received some substantial feedback on #95290, which I plan to address
in a future PR.

I've also received feedback that because the change emits errors where
they were not emitted before, we should at least have a flag to disable
the stricter warnings.

show more ...

[clang][ThreadSafety] Check trylock function success and return types (#95290)

With this change, Clang will generate errors when trylock functions have
improper return types. Today, it silently fai

[clang][ThreadSafety] Check trylock function success and return types (#95290)

With this change, Clang will generate errors when trylock functions have
improper return types. Today, it silently fails to apply the trylock
attribute to these functions which may incorrectly lead users to believe
they have correctly acquired locks before accessing guarded data.

As a side effect of explicitly checking the success argument type, I
seem to have fixed a false negative in the analysis that could occur
when a trylock's success argument is an enumerator. I've added a
regression test to warn-thread-safety-analysis.cpp named
`TrylockSuccessEnumFalseNegative`.

This change also improves the documentation with descriptions of of the
subtle gotchas that arise from the analysis interpreting the success arg
as a boolean.

Issue #92408

show more ...

Thread safety analysis: Fix a bug in handling temporary constructors (#74020)

Extends the lifetime of the map `ConstructedObjects` to be of the
whole CFG so that the map can connect temporary Ctor

Thread safety analysis: Fix a bug in handling temporary constructors (#74020)

Extends the lifetime of the map `ConstructedObjects` to be of the
whole CFG so that the map can connect temporary Ctor and Dtor in
different CFG blocks.

show more ...

Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (#68572)

…… (#68394)"

The new warnings are now under a separate flag
`-Wthread-safety-reference-return`, which is on by def

Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (#68572)

…… (#68394)"

The new warnings are now under a separate flag
`-Wthread-safety-reference-return`, which is on by default under
`-Wthread-safety-reference`.

- People can opt out via `-Wthread-safety-reference
-Wnothread-safety-reference-return`.
This reverts commit 859f2d032386632562521a99db20923217d98988.

show more ...

Revert "Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (#68394)"

This reverts commit cd184c866e0aad1f957910b8c7a94f98a2b21ceb.

Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (#68394)

…. (#67776)"

Now that `scudo` issues have been fixed (#68273).

This reverts commit 462bdd5bf0861a27f451f7917802a

Reapply "[clang analysis][thread-safety] Handle return-by-reference..… (#68394)

…. (#67776)"

Now that `scudo` issues have been fixed (#68273).

This reverts commit 462bdd5bf0861a27f451f7917802a954e2046bc7.

show more ...

Revert "[clang analysis][thread-safety] Handle return-by-reference...… (#67795)

… (#67776)"

This detects issues in `scudo`. Reverting until these are fixed.

```
/b/sanitizer-x86_64-linux-auto

Revert "[clang analysis][thread-safety] Handle return-by-reference...… (#67795)

… (#67776)"

This detects issues in `scudo`. Reverting until these are fixed.

```
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd.h:74:12: error: returning variable 'QuarantineCache' by reference requires holding mutex 'Mutex' exclusively [-Werror,-Wthread-safety-reference]
74 | return QuarantineCache;
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/combined.h:248:28: note: in instantiation of member function 'scudo::TSD<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::getQuarantineCache' requested here
248 | Quarantine.drain(&TSD->getQuarantineCache(),
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd.h:57:15: note: in instantiation of member function 'scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>::commitBack' requested here
57 | Instance->commitBack(this);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd_exclusive.h:172:27: note: in instantiation of member function 'scudo::TSD<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::commitBack' requested here
172 | TSDRegistryT::ThreadTSD.commitBack(Instance);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd_exclusive.h:33:46: note: in instantiation of function template specialization 'scudo::teardownThread<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>' requested here
33 | CHECK_EQ(pthread_key_create(&PThreadKey, teardownThread<Allocator>), 0);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd_exclusive.h:42:5: note: in instantiation of member function 'scudo::TSDRegistryExT<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::init' requested here
42 | init(Instance); // Sets Initialized.
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd_exclusive.h:130:5: note: in instantiation of member function 'scudo::TSDRegistryExT<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::initOnceMaybe' requested here
130 | initOnceMaybe(Instance);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/tsd_exclusive.h:74:5: note: in instantiation of member function 'scudo::TSDRegistryExT<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::initThread' requested here
74 | initThread(Instance, MinimalInit);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/combined.h:221:17: note: in instantiation of member function 'scudo::TSDRegistryExT<scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>>::initThreadMaybe' requested here
221 | TSDRegistry.initThreadMaybe(this, MinimalInit);
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/combined.h:790:5: note: in instantiation of member function 'scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>::initThreadMaybe' requested here
790 | initThreadMaybe();
| ^
/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/scudo/standalone/wrappers_c.inc:36:25: note: in instantiation of member function 'scudo::Allocator<scudo::DefaultConfig, &malloc_postinit>::canReturnNull' requested here
36 | if (SCUDO_ALLOCATOR.canReturnNull()) {
```

This reverts commit 6dd96d6e80e9b3679a6161c590c60e0e99549b89.

show more ...

[clang analysis][thread-safety] Handle return-by-reference... (#67776)

...of guarded variables, when the function is not marked as requiring
locks:

```
class Return {
Mutex mu;
Foo foo GU

[clang analysis][thread-safety] Handle return-by-reference... (#67776)

...of guarded variables, when the function is not marked as requiring
locks:

```
class Return {
Mutex mu;
Foo foo GUARDED_BY(mu);

Foo &returns_ref_locked() {
MutexLock lock(&mu);
return foo; // BAD
}

Foo &returns_ref_locks_required() SHARED_LOCKS_REQUIRED(mu) {
return foo; // OK
}
};
```

Review on Phabricator: https://reviews.llvm.org/D153131

show more ...

Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… (#67775)

…" (#67523)

Discussion in https://reviews.llvm.org/D153132.

This reverts commit f70377471c990aa567584ae429e77adc

Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… (#67775)

…" (#67523)

Discussion in https://reviews.llvm.org/D153132.

This reverts commit f70377471c990aa567584ae429e77adc9a55491b.

show more ...

Revert "[clang analysis][NFCI] Preparatory work for D153131. (#67420)" (#67523)

There was a misunderstanding as to whether we wanted those base NFC
changes or not.

This reverts commit 166074eff2

Revert "[clang analysis][NFCI] Preparatory work for D153131. (#67420)" (#67523)

There was a misunderstanding as to whether we wanted those base NFC
changes or not.

This reverts commit 166074eff2e9a5f79b791f1cc9b641a4e2968616.

show more ...

[clang analysis][NFCI] Preparatory work for D153131. (#67420)

This was ported over from phabricator.

Review https://reviews.llvm.org/D153131.

[clang][TSA] Thread safety cleanup functions

Consider cleanup functions in thread safety analysis.

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

[NFC] Preparatory work for D153131 (#66750)

[NFC] Initialize member pointer and avoid potential null dereference

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

Revert "[clang][CFG][NFC] A few smaller cleanups"

This reverts commit 173df3dd5f9a812b07f9866965f4e92a982a3fca.

Looks like this wasn't as innocent as it seemed:
https://lab.llvm.org/buildbot#builde

Revert "[clang][CFG][NFC] A few smaller cleanups"

This reverts commit 173df3dd5f9a812b07f9866965f4e92a982a3fca.

Looks like this wasn't as innocent as it seemed:
https://lab.llvm.org/buildbot#builders/38/builds/12982

show more ...

[clang][CFG][NFC] A few smaller cleanups

Use dyn_cast_if_present instead of _or_null, use decomposition decls,
and a few other minor things.

[CLANG] Fix potential null pointer dereference bugs

This patch uses castAs instead of getAs which will assert if the type doesn't match and adds nullptr check if needed.

Also this patch improves th

[CLANG] Fix potential null pointer dereference bugs

This patch uses castAs instead of getAs which will assert if the type doesn't match and adds nullptr check if needed.

Also this patch improves the codes and passes I.getData() instead of doing a lookup in dumpVarDefinitionName()
since we're iterating over the same map in LocalVariableMap::dumpContex().

Reviewed By: aaron.ballman, aaronpuchert

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

show more ...

[clang][ThreadSafety][NFC] Make isReference() const

[clang] Use *{Set,Map}::contains (NFC)

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is p

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Option

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Optional with
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...