aaron.ballman added a comment. In D152246#4412333 <https://reviews.llvm.org/D152246#4412333>, @aaronpuchert wrote:
> 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. That's good to know, thank you for bringing this up! I agree that we likely don't want to do alias analysis. The way `CallExpr` implements `getCalleeDecl()` is to check whether the callee expression (after ignoring implicit casts and dereference operators) references a declaration of some kind (`DeclRefExpr`, `MemberExpr`, `BlockExpr`) and if so, report back the declaration found. So it does not do any sort of tricky dataflow analysis and only handles relatively simple cases like: `void func() {} void (*fp)() = func; fp();` > 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 something > along the lines of "we'd really like to integrate this into the type system, > but it would be quite difficult". Yeah, I seem to recall this adding enough overhead to the type system for it to be problematic. I think we were lamenting that Clang doesn't have a pluggable type system and we'd love to see one added, but that's a big, experimental undertaking. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152246/new/ https://reviews.llvm.org/D152246 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
