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()
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()
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits