Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1901,6 +1901,29 @@ Check the size argument passed into C string functions for common erroneous patt

.. _unix-cstring-NullArg:

.. _alpha-unix-cstring-NotNullTerminated:

unix.cstring.NotNullTerminated (C)
""""""""""""""""""""""""""""""""""
Check for arguments which are not null-terminated strings;
applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of functions.

Only very fundamental cases are detected where the passed memory block is
absolutely different from a null-terminated string. This checker does not
find if a memory buffer is passed where the terminating zero character
is missing.

.. code-block:: c

void test1() {
int l = strlen((char *)&test); // warn
}

void test2() {
label:
int l = strlen((char *)&&label); // warn
}

unix.cstring.NullArg (C)
""""""""""""""""""""""""
Check for null pointers being passed as arguments to C string functions:
Expand Down Expand Up @@ -3367,29 +3390,6 @@ Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmem
memcpy(a + 2, a + 1, 8); // warn
}

.. _alpha-unix-cstring-NotNullTerminated:

alpha.unix.cstring.NotNullTerminated (C)
""""""""""""""""""""""""""""""""""""""""
Check for arguments which are not null-terminated strings;
applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of functions.

Only very fundamental cases are detected where the passed memory block is
absolutely different from a null-terminated string. This checker does not
find if a memory buffer is passed where the terminating zero character
is missing.

.. code-block:: c

void test1() {
int l = strlen((char *)&test); // warn
}

void test2() {
label:
int l = strlen((char *)&&label); // warn
}

.. _alpha-unix-cstring-OutOfBounds:

alpha.unix.cstring.OutOfBounds (C)
Expand Down
11 changes: 6 additions & 5 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,12 @@ def CStringModeling : Checker<"CStringModeling">,
Documentation<NotDocumented>,
Hidden;

def CStringNotNullTerm : Checker<"NotNullTerminated">,
HelpText<"Check for arguments passed to C string functions which are not "
"null-terminated strings">,
Dependencies<[CStringModeling]>,
Documentation<HasDocumentation>;

def CStringNullArg : Checker<"NullArg">,
HelpText<"Check for null pointers being passed as arguments to C string "
"functions">,
Expand All @@ -485,11 +491,6 @@ def CStringBufferOverlap : Checker<"BufferOverlap">,
Dependencies<[CStringModeling]>,
Documentation<HasDocumentation>;

def CStringNotNullTerm : Checker<"NotNullTerminated">,
HelpText<"Check for arguments which are not null-terminating strings">,
Dependencies<[CStringModeling]>,
Documentation<HasDocumentation>;

def CStringUninitializedRead : Checker<"UninitializedRead">,
HelpText<"Checks if the string manipulation function would read uninitialized bytes">,
Dependencies<[CStringModeling]>,
Expand Down
1 change: 1 addition & 0 deletions clang/test/Analysis/analyzer-enabled-checkers.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
// CHECK-NEXT: unix.StdCLibraryFunctions
// CHECK-NEXT: unix.Vfork
// CHECK-NEXT: unix.cstring.BadSizeArg
// CHECK-NEXT: unix.cstring.NotNullTerminated
// CHECK-NEXT: unix.cstring.NullArg

int main() {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/bstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,unix.cstring.NotNullTerminated,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s

#include "Inputs/system-header-simulator-cxx.h"
#include "Inputs/system-header-simulator-for-malloc.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
// CHECK-NEXT: unix.StdCLibraryFunctions
// CHECK-NEXT: unix.Vfork
// CHECK-NEXT: unix.cstring.BadSizeArg
// CHECK-NEXT: unix.cstring.NotNullTerminated
// CHECK-NEXT: unix.cstring.NullArg

int main() {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/string.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
// RUN: -analyzer-checker=unix.cstring \
// RUN: -analyzer-checker=unix.Malloc \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap \
// RUN: -analyzer-checker=alpha.unix.cstring.NotNullTerminated \
// RUN: -analyzer-checker=unix.cstring.NotNullTerminated \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false

Expand Down
4 changes: 4 additions & 0 deletions clang/test/Analysis/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,7 @@ struct TestNotNullTerm {
strlen((char *)&x); // expected-warning{{Argument to string length function is not a null-terminated string}}
}
};

void test_notcstring_tempobject() {
strlen((char[]){'a', 0}); // expected-warning{{Argument to string length function is a C++ temp object of type char[2], which is not a null-terminated string}}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this result looks like a false positive (I'd guess that this strlen call is well-defined and returns 1, but I'm not familiar with the exact rules for C++ temp objects).

If this is indeed a false positive, then either fix it, or add a comment like "FIXME: In this corner case the checker produces a false positive".

If the report is a true positive, then perhaps explain why is it a true positive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is taken from a FIXME in line 991 of the checker code. This exact case is really a false positive, a warning is needed only if a write happens into such a region. For this exact case (temporary object that behaves like a C string) the checker can be improved. But such a change would affect other checkers in the 'cstring' group.

Loading