NoQ added a comment.
I've seen this recently, and while i agree that the fix is correct, i'm not
entirely sure that the test cases are correct. As weird as this may sound, null
dereference is not an attempt to read from or write to memory address 0.
Instead, it is about using a null pointer as if it was pointing to an actual
object in memory, even if accessing it by a non-zero offset. For example, in
struct S {
int x, y;
};
void foo() {
struct S *s = NULL;
s->y = 1;
}
we're in fact writing into `*(0x4)`, not `*(0x0)`, however it's intuitive that
this code has a //null// pointer dereference, because we use a null pointer `s`
as if it points to an actual object of type `struct S`. In this sense, i'd
actually want the analyzer to warn in `test4`: it seems that the author of this
code was expecting to find something useful by offset 1 to the pointer, so he
must have made a mistake. Also i'm not entirely sure if i want the analyzer to
warn in `test1`, `test2`, `test3` (also this code pattern doesn't look
widespread/idiomatic).
So the analyzer does this really really weird thing by treating many operations
with concrete pointers as no-ops, keeping the pointer null when it was null
before, and keeping it non-null when it was non-null before - not just in the
place that you've fixed, but also when computing field or element or base-class
offsets for null pointers. These are marked as FIXME all over the place, but
taking up the fix would require to provide another heuristic to distinguish
null dereferences from other fixed-address dereferences (i.e. the experimental
`FixedAddressChecker`).
I guess there should be more comments on this issue near the FIXMEs you fixed,
and more tests covering the intended behavior; adding them is definitely a good
thing to do. I do not have any immediate ideas on how to fix the issue as a
whole.
================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:938
llvm::APSInt Multiplicand(rightI.getBitWidth(), /* isUnsigned */ true);
+ QualType PteeTy =
resultTy.getTypePtr()->castAs<PointerType>()->getPointeeType();
+ Multiplicand = getContext().getTypeSizeInChars(PteeTy).getQuantity();
----------------
xazax.hun wrote:
> The rest of the code does not abbreviate the Type. I would prefer to name
> this `pointeeType`.
Also `resultTy->getPointeeType()`. Note the fancy `operator->()` in `QualType`.
https://reviews.llvm.org/D37478
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
