NoQ added a comment.
That's a fix for https://bugs.llvm.org/show_bug.cgi?id=46264.
Your code looks great but i don't understand at a glance what the crash was
caused by and how does your code fix it, can you explain? Like, the original
test doesn't have any `void *`s and it doesn't have any indirect fields. Also
the crash happens during visitor phase but the fixes are entirely during
modeling phase.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2533-2534
if (isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D)) {
- // FIXME: Compute lvalue of field pointers-to-member.
- // Right now we just use a non-null void pointer, so that it gives proper
- // results in boolean contexts.
- // FIXME: Maybe delegate this to the surrounding operator&.
- // Note how this expression is lvalue, however pointer-to-member is NonLoc.
- SVal V = svalBuilder.conjureSymbolVal(Ex, LCtx, getContext().VoidPtrTy,
- currBldrCtx->blockCount());
- state = state->assume(V.castAs<DefinedOrUnknownSVal>(), true);
- Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
- ProgramPoint::PostLValueKind);
+ // Delegate all work related to pointer to members to the surrounding
+ // operator&.
return;
----------------
Ok so you're saying that there's *always* going to be a surrounding operator
`&`? That kind of makes sense but if you add more explanation/proof of how you
figured this out that'd be great.
================
Comment at: clang/test/Analysis/PR46264.cpp:5
+
+namespace a {
+class b {
----------------
I'm pretty sure the namespace is not actually important for the test. `creduce`
is great but sometimes it misses stuff. Generally, i'm a big fan of manually
cleaning up `creduce`d test to make them look more like real code. At least
turn things like `b h;` and `e *i;` into something like `A a;` and `B *b;`.
Also replace `class` with `struct` because we never care about this entire
public/private business and `struct` provides a nice default.
================
Comment at: clang/test/Analysis/PR46264.cpp:9
+ typedef int b::*c;
+ operator c() { return &b::d; }
+ int d;
----------------
It's worth it to comment where exactly is this conversion operator used in the
text (i.e., within the if-condition). People do in fact sometimes do this kind
of thing in real life: provide a conversion to pointer-to-member *instead of*
conversion to `bool` because it causes fewer potential further accidental
implicit conversions to be possible.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85817/new/
https://reviews.llvm.org/D85817
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
