Author: einvbri Date: 2022-07-13T19:19:23-05:00 New Revision: 1d7e58cfade1dd4236efba59ce969698bb05a039
URL: https://github.com/llvm/llvm-project/commit/1d7e58cfade1dd4236efba59ce969698bb05a039 DIFF: https://github.com/llvm/llvm-project/commit/1d7e58cfade1dd4236efba59ce969698bb05a039.diff LOG: [analyzer] Fix use of length in CStringChecker CStringChecker is using getByteLength to get the length of a string literal. For targets where a "char" is 8-bits, getByteLength() and getLength() will be equal for a C string, but for targets where a "char" is 16-bits getByteLength() returns the size in octets. This is verified in our downstream target, but we have no way to add a test case for this case since there is no target supporting 16-bit "char" upstream. Since this cannot have a test case, I'm asserted this change is "correct by construction", and visually inspected to be correct by way of the following example where this was found. The case that shows this fails using a target with 16-bit chars is here. getByteLength() for the string literal returns 4, which fails when checked against "char x[4]". With the change, the string literal is evaluated to a size of 2 which is a correct number of "char"'s for a 16-bit target. ``` void strcpy_no_overflow_2(char *y) { char x[4]; strcpy(x, "12"); // with getByteLength(), returns 4 using 16-bit chars } ``` This change exposed that embedded nulls within the string are not handled. This is documented as a FIXME for a future fix. ``` void strcpy_no_overflow_3(char *y) { char x[3]; strcpy(x, "12\0"); } ``` Reviewed By: martong Differential Revision: https://reviews.llvm.org/D129269 Added: Modified: clang/docs/analyzer/checkers.rst clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp clang/test/Analysis/string.c Removed: ################################################################################ diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 6f2486fb49dfc..da53fad14bc57 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2726,6 +2726,9 @@ alpha.unix.cstring.OutOfBounds (C) """""""""""""""""""""""""""""""""" Check for out-of-bounds access in string functions; applies to:`` strncopy, strncat``. +This check also applies to string literals, except there is a known bug in that +the analyzer cannot detect embedded NULL characters. + .. code-block:: c void test() { diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 2e4c8e6436988..987cf65d6fec6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -848,7 +848,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, SValBuilder &svalBuilder = C.getSValBuilder(); QualType sizeTy = svalBuilder.getContext().getSizeType(); const StringLiteral *strLit = cast<StringRegion>(MR)->getStringLiteral(); - return svalBuilder.makeIntVal(strLit->getByteLength(), sizeTy); + return svalBuilder.makeIntVal(strLit->getLength(), sizeTy); } case MemRegion::SymbolicRegionKind: case MemRegion::AllocaRegionKind: diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c index d1fcd92e73270..6bdf59c56f63b 100644 --- a/clang/test/Analysis/string.c +++ b/clang/test/Analysis/string.c @@ -1652,3 +1652,18 @@ void test_memset_chk(void) { __builtin___memset_chk(&x, 0, sizeof(x), __builtin_object_size(&x, 0)); clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} } + +#ifndef SUPPRESS_OUT_OF_BOUND +void strcpy_no_overflow_2(char *y) { + char x[3]; + // FIXME: string literal modeling does not account for embedded NULLs. + // This case should not elicit a warning, but does. + // See discussion at https://reviews.llvm.org/D129269 + strcpy(x, "12\0"); // expected-warning{{String copy function overflows the destination buffer}} +} +#else +void strcpy_no_overflow_2(char *y) { + char x[3]; + strcpy(x, "12\0"); +} +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits