This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Track null or undef values through pointer arithmetic.
ClosedPublic

Authored by NoQ on Mar 29 2018, 5:16 PM.

Details

Summary

Pointer arithmetic on null or undefined pointers results in null or undefined pointers. This is obvious for undefined pointers; for null pointers it follows from our incorrect-but-somehow-working approach that declares that 0 (Loc) doesn't necessarily represent a pointer of numeric address value 0, but instead it represents any pointer that will cause a valid "null pointer dereference" issue when dereferenced.

For now we've been seeing through pointer arithmetic at the original dereference expression, i.e. in bugreporter::getDerefExpr(), but not during further investigation of the value's origins in bugreporter::trackNullOrUndefValue(). The patch fixes it.

Reproducers are reduced from WebKit's custom string implementations. CStringChecker doesn't call getDerefExpr() at all (this is probably because passing a pointer to memcpy is not an immediate null dereference - in fact i've no idea how this decision was made or how this code was actually supposed to operate). But the second test case demonstrates that simply adding getDerefExpr() to CStringChecker is insufficient. Also an older FIXME test gets fixed this way.

Diff Detail

Event Timeline

NoQ created this revision.Mar 29 2018, 5:16 PM

LGTM with a nit.
Also I don't quite understand why being additive is important? Isn't pointer subtraction basically the same?

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
78

static.
+1 for using functions.

This revision is now accepted and ready to land.Mar 30 2018, 10:41 AM
NoQ updated this revision to Diff 140466.Mar 30 2018, 12:11 PM

Substraction is an additive operation. Added tests for that. Added even more tests.

NoQ marked an inline comment as done.Mar 30 2018, 12:11 PM
NoQ added inline comments.
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
78

Whoops.

This revision was automatically updated to reflect the committed changes.