[PATCH] D152246: [clang][ThreadSafety] Analyze known function pointer values

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

That's a tough one. The change seems to be correct, and makes more patterns visible. Of course I wonder how it comes that we see calls of a function pointer with a uniquely determined value, as that would seem like obfuscation. Maybe you can show an example how this might look like in practice?

Another issue (that of course you can't know about) is that I'm not sure about the `LocalVariableMap`. It's currently only used to find a try-acquire function return value from a branch, not in the core analysis and can apparently be quite expensive <https://github.com/llvm/llvm-project/issues/42656> for long functions with exception handling edges. I'd like to get rid of it, unless we decide to use it for resolving aliases more widely. But I'm wary of alias analysis, because we don't want to reimplement the static analyzer, and we also want to be more predictable, maybe even provide correctness guarantees. Alias analysis ends in undecidable territory quite early on, and using heuristics would sacrifice predictability.

The relatively strict "dumb" analysis that we're doing today seems to work well enough for the purpose of tracking locks, though we can look through local references. (Their pointee doesn't change. Though it doesn't always work <https://github.com/llvm/llvm-project/issues/62497>.) My hope would be that we can leave it at that and don't build a fully-fledged alias analysis.

I'd like if we could address the issue here by moving to type attributes. These would of course be "sugar", i.e. dropped on canonicalization, as we don't want them to affect overloading etc. But that would still allow more than declaration attributes, like in this case, warning if we drop the attribute on assignment. Of course that's big project though, and it's not clear if it'll work out. The paper <https://static.googleusercontent.com/media/research.google.com/en/us/pubs/archive/42958.pdf> by @delesley and @aaron.ballman briefly says that non-sugared attributes would probably not work, and there is a talk <https://www.youtube.com/watch?v=5Xx0zktqSs4> where @delesley says basically "we'd really like to integrate this into the type system, but it would be quite difficult".


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

https://reviews.llvm.org/D152246