ziqingluo-90 wrote:
I see. I forgot about the `unsafe_alias(...)` trick, which relies on
invalidation. I agree we need this suppression trick to make TSA useful. So
the motivation for the follow-up PR makes sense to me.
- >In particular, I didn't like that we effectively undid what
testPointerAliasEscapeMultiple() was testing ...
- >// missing unlock - generic realloction + copy would be a data race ...
These are valid concerns to me. We have internal users of TSA. Speaking on
behalf of them, we believe it would be ideal to have the invalidation
assumption accompanied by annotations that mark exceptions and can be checked
on the implementation side. Annotation burden is not a big issue as long as we
move toward soundness and fewer false positives.
The example from `core.c` coincides with what I've been thinking per our
internal users' request.
```
context_unsafe_alias(rq);
...
rq_lock(rq, &rf);
...
if (task_rq(p) == rq && task_on_rq_queued(p))
rq = __migrate_task(rq, &rf, p, cpu);
rq_unlock(rq, &rf);
```
Ideally, we want to make the code truly safe by annotating `__migrate_task`
with the following properties (I'm assuming a simple model for
`__migrate_task`):
1. the argument `rq` will be unlocked; and
2. the returned value will point to a locked object.
With existing annotations, we can achieve 1. For 2, we have something working
but it is not sound at call sites, and return values are not handled at the
implementation side.
https://github.com/llvm/llvm-project/pull/187691
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
