Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -194,6 +194,14 @@ const Stmt *First, const Stmt *Second) const; + void emitNullArgBug(CheckerContext &C, ProgramStateRef State, const Stmt *S, + StringRef WarningMsg) const; + void emitOutOfBoundsBug(CheckerContext &C, ProgramStateRef State, + const Stmt *S, StringRef WarningMsg) const; + void emitNotCStringBug(CheckerContext &C, ProgramStateRef State, + const Stmt *S, StringRef WarningMsg) const; + void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const; + ProgramStateRef checkAdditionOverflow(CheckerContext &C, ProgramStateRef state, NonLoc left, @@ -239,30 +247,14 @@ std::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType()); if (stateNull && !stateNonNull) { - if (!Filter.CheckCStringNullArg) - return nullptr; - - ExplodedNode *N = C.generateErrorNode(stateNull); - if (!N) - return nullptr; - - if (!BT_Null) - BT_Null.reset(new BuiltinBug( - Filter.CheckNameCStringNullArg, categories::UnixAPI, - "Null pointer argument in call to byte string function")); - - SmallString<80> buf; - llvm::raw_svector_ostream os(buf); - assert(CurrentFunctionDescription); - os << "Null pointer argument in call to " << CurrentFunctionDescription; - - // Generate a report for this bug. - BuiltinBug *BT = static_cast(BT_Null.get()); - auto report = llvm::make_unique(*BT, os.str(), N); + if (Filter.CheckCStringNullArg) { + SmallString<80> buf; + llvm::raw_svector_ostream os(buf); + assert(CurrentFunctionDescription); + os << "Null pointer argument in call to " << CurrentFunctionDescription; - report->addRange(S->getSourceRange()); - bugreporter::trackNullOrUndefValue(N, S, *report); - C.emitReport(std::move(report)); + emitNullArgBug(C, stateNull, S, os.str()); + } return nullptr; } @@ -305,31 +297,14 @@ ProgramStateRef StInBound = state->assumeInBound(Idx, Size, true); ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false); if (StOutBound && !StInBound) { - ExplodedNode *N = C.generateErrorNode(StOutBound); - if (!N) - return nullptr; - - CheckName Name; // These checks are either enabled by the CString out-of-bounds checker // explicitly or the "basic" CStringNullArg checker support that Malloc // checker enables. assert(Filter.CheckCStringOutOfBounds || Filter.CheckCStringNullArg); - if (Filter.CheckCStringOutOfBounds) - Name = Filter.CheckNameCStringOutOfBounds; - else - Name = Filter.CheckNameCStringNullArg; - if (!BT_Bounds) { - BT_Bounds.reset(new BuiltinBug( - Name, "Out-of-bound array access", - "Byte string function accesses out-of-bound array element")); - } - BuiltinBug *BT = static_cast(BT_Bounds.get()); - - // Generate a report for this bug. - std::unique_ptr report; + // Emit a bug report. if (warningMsg) { - report = llvm::make_unique(*BT, warningMsg, N); + emitOutOfBoundsBug(C, StOutBound, S, warningMsg); } else { assert(CurrentFunctionDescription); assert(CurrentFunctionDescription[0] != '\0'); @@ -339,15 +314,8 @@ os << toUppercase(CurrentFunctionDescription[0]) << &CurrentFunctionDescription[1] << " accesses out-of-bound array element"; - report = llvm::make_unique(*BT, os.str(), N); + emitOutOfBoundsBug(C, StOutBound, S, os.str()); } - - // FIXME: It would be nice to eventually make this diagnostic more clear, - // e.g., by referencing the original declaration or by saying *why* this - // reference is outside the range. - - report->addRange(S->getSourceRange()); - C.emitReport(std::move(report)); return nullptr; } @@ -565,6 +533,79 @@ C.emitReport(std::move(report)); } +void CStringChecker::emitNullArgBug(CheckerContext &C, ProgramStateRef State, + const Stmt *S, StringRef WarningMsg) const { + if (ExplodedNode *N = C.generateErrorNode(State)) { + if (!BT_Null) + BT_Null.reset(new BuiltinBug( + Filter.CheckNameCStringNullArg, categories::UnixAPI, + "Null pointer argument in call to byte string function")); + + BuiltinBug *BT = static_cast(BT_Null.get()); + auto Report = llvm::make_unique(*BT, WarningMsg, N); + bugreporter::trackNullOrUndefValue(N, S, *Report); + C.emitReport(std::move(Report)); + } +} + +void CStringChecker::emitOutOfBoundsBug(CheckerContext &C, + ProgramStateRef State, const Stmt *S, + StringRef WarningMsg) const { + if (ExplodedNode *N = C.generateErrorNode(State)) { + if (!BT_Bounds) + BT_Bounds.reset(new BuiltinBug( + Filter.CheckCStringOutOfBounds ? Filter.CheckNameCStringOutOfBounds + : Filter.CheckNameCStringNullArg, + "Out-of-bound array access", + "Byte string function accesses out-of-bound array element")); + + BuiltinBug *BT = static_cast(BT_Bounds.get()); + + // FIXME: It would be nice to eventually make this diagnostic more clear, + // e.g., by referencing the original declaration or by saying *why* this + // reference is outside the range. + auto Report = llvm::make_unique(*BT, WarningMsg, N); + Report->addRange(S->getSourceRange()); + C.emitReport(std::move(Report)); + } +} + +void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State, + const Stmt *S, + StringRef WarningMsg) const { + if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { + if (!BT_NotCString) + BT_NotCString.reset(new BuiltinBug( + Filter.CheckNameCStringNotNullTerm, categories::UnixAPI, + "Argument is not a null-terminated string.")); + + auto Report = llvm::make_unique(*BT_NotCString, WarningMsg, N); + + Report->addRange(S->getSourceRange()); + C.emitReport(std::move(Report)); + } +} + +void CStringChecker::emitAdditionOverflowBug(CheckerContext &C, + ProgramStateRef State) const { + if (ExplodedNode *N = C.generateErrorNode(State)) { + if (!BT_NotCString) + BT_NotCString.reset( + new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API", + "Sum of expressions causes overflow.")); + + // This isn't a great error message, but this should never occur in real + // code anyway -- you'd have to create a buffer longer than a size_t can + // represent, which is sort of a contradiction. + const char *WarningMsg = + "This expression will create a string whose length is too big to " + "be represented as a size_t"; + + auto Report = llvm::make_unique(*BT_NotCString, WarningMsg, N); + C.emitReport(std::move(Report)); + } +} + ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, ProgramStateRef state, NonLoc left, @@ -608,26 +649,7 @@ if (stateOverflow && !stateOkay) { // We have an overflow. Emit a bug report. - ExplodedNode *N = C.generateErrorNode(stateOverflow); - if (!N) - return nullptr; - - if (!BT_AdditionOverflow) - BT_AdditionOverflow.reset( - new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API", - "Sum of expressions causes overflow")); - - // This isn't a great error message, but this should never occur in real - // code anyway -- you'd have to create a buffer longer than a size_t can - // represent, which is sort of a contradiction. - const char *warning = - "This expression will create a string whose length is too big to " - "be represented as a size_t"; - - // Generate a report for this bug. - C.emitReport( - llvm::make_unique(*BT_AdditionOverflow, warning, N)); - + emitAdditionOverflowBug(C, stateOverflow); return nullptr; } @@ -727,15 +749,7 @@ // C string. In the context of locations, the only time we can issue such // a warning is for labels. if (Optional Label = Buf.getAs()) { - if (!Filter.CheckCStringNotNullTerm) - return UndefinedVal(); - - if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) { - if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug( - Filter.CheckNameCStringNotNullTerm, categories::UnixAPI, - "Argument is not a null-terminated string.")); - + if (Filter.CheckCStringNotNullTerm) { SmallString<120> buf; llvm::raw_svector_ostream os(buf); assert(CurrentFunctionDescription); @@ -743,14 +757,9 @@ << " is the address of the label '" << Label->getLabel()->getName() << "', which is not a null-terminated string"; - // Generate a report for this bug. - auto report = llvm::make_unique(*BT_NotCString, os.str(), N); - - report->addRange(Ex->getSourceRange()); - C.emitReport(std::move(report)); + emitNotCStringBug(C, state, Ex, os.str()); } return UndefinedVal(); - } // If it's not a region and not a label, give up. @@ -787,15 +796,7 @@ // Other regions (mostly non-data) can't have a reliable C string length. // In this case, an error is emitted and UndefinedVal is returned. // The caller should always be prepared to handle this case. - if (!Filter.CheckCStringNotNullTerm) - return UndefinedVal(); - - if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) { - if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug( - Filter.CheckNameCStringNotNullTerm, categories::UnixAPI, - "Argument is not a null-terminated string.")); - + if (Filter.CheckCStringNotNullTerm) { SmallString<120> buf; llvm::raw_svector_ostream os(buf); @@ -807,13 +808,8 @@ else os << "not a null-terminated string"; - // Generate a report for this bug. - auto report = llvm::make_unique(*BT_NotCString, os.str(), N); - - report->addRange(Ex->getSourceRange()); - C.emitReport(std::move(report)); + emitNotCStringBug(C, state, Ex, os.str()); } - return UndefinedVal(); } }