[PATCH] D39862: [analyzer] do not crash when trying to convert an APSInt to an unexpected type

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun.

This is the issue breaking the postgresql bot, purely by chance exposed through taint checker, somehow appearing after https://reviews.llvm.org/D38358 got committed.

The backstory is that the taint checker requests `SVal` for the value of the pointer, and analyzer has a "fast path" in the getter to return a constant when we know that the value is constant.
Unfortunately, the getter requires a cast to get signedness correctly, and for the pointer `void *` the cast crashes.

This is more of a band-aid patch, as I am not sure what could be done here "correctly", but it should be applied in any case to avoid the crash.

@dcoughlin faster review here would be appreciated to get the bot back to green.


https://reviews.llvm.org/D39862

Files:
  lib/StaticAnalyzer/Core/ProgramState.cpp
  test/Analysis/taint-tester.c


Index: test/Analysis/taint-tester.c
===================================================================
--- test/Analysis/taint-tester.c
+++ test/Analysis/taint-tester.c
@@ -189,3 +189,10 @@
 
 }
 
+char *pointer1;
+void *pointer2;
+void noCrashTest() {
+  if (!*pointer1) {
+    __builtin___memcpy_chk(pointer2, pointer1, 0, 0); // no-crash
+  }
+}
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -260,7 +260,9 @@
   // be a constant value, use that value instead to lessen the burden
   // on later analysis stages (so we have less symbolic values to reason
   // about).
-  if (!T.isNull()) {
+  // We only go into this branch if we can convert the APSInt value we have
+  // to the type of T, which is not always the case (e.g. for void).
+  if (!T.isNull() && (T->isIntegralOrEnumerationType() || Loc::isLocType(T))) {
     if (SymbolRef sym = V.getAsSymbol()) {
       if (const llvm::APSInt *Int = getStateManager()
                                     .getConstraintManager()


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39862.122306.patch
Type: text/x-patch
Size: 1145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171109/ca12eb84/attachment.bin>