diff options
author | Etienne Bergeron <etienneb@google.com> | 2016-04-15 16:36:00 +0000 |
---|---|---|
committer | Etienne Bergeron <etienneb@google.com> | 2016-04-15 16:36:00 +0000 |
commit | 1f696b316c705ec7236b3a14e7f5b274de5535c7 (patch) | |
tree | c9b9d7696dcec3cb88f02c56838408b130403860 /clang-tools-extra/docs/clang-tidy | |
parent | 3c5be6c9a72bad003348459a578b6b37678b075e (diff) | |
download | bcm5719-llvm-1f696b316c705ec7236b3a14e7f5b274de5535c7.tar.gz bcm5719-llvm-1f696b316c705ec7236b3a14e7f5b274de5535c7.zip |
[clang-tidy] Add new checker for suspicious sizeof expressions
Summary:
This check is finding suspicious cases of sizeof expression.
Sizeof expression is returning the size (in bytes) of a type or an
expression. Programmers often abuse or misuse this expression.
This checker is adding common set of patterns to detect some
of these bad constructs.
Some examples found by this checker:
R/packages/ifultools/ifultools/src/fra_neig.c
```
/* free buffer memory */
(void) mutil_free( dist_buff, sizeof( ctr * sizeof( double ) ) );
(void) mutil_free( nidx_buff, sizeof( ctr * sizeof( sint32 ) ) );
```
graphviz/v2_20_2/lib/common/utils.c
```
static Dtdisc_t mapDisc = {
offsetof(item, p),
sizeof(2 * sizeof(void *)),
offsetof(item, link),
(Dtmake_f) newItem,
(Dtfree_f) freeItem,
(Dtcompar_f) cmpItem,
NIL(Dthash_f),
NIL(Dtmemory_f),
NIL(Dtevent_f)
};
```
mDNSResponder/mDNSShared/dnsextd.c
```
context = ( TCPContext* ) malloc( sizeof( TCPContext ) );
require_action( context, exit, err = mStatus_NoMemoryErr; LogErr( "AcceptTCPConnection", "malloc" ) );
mDNSPlatformMemZero( context, sizeof( sizeof( TCPContext ) ) );
context->d = self;
```
Reviewers: alexfh
Subscribers: malcolm.parsons, Eugene.Zelenko, cfe-commits
Differential Revision: http://reviews.llvm.org/D19014
llvm-svn: 266451
Diffstat (limited to 'clang-tools-extra/docs/clang-tidy')
-rw-r--r-- | clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 | ||||
-rw-r--r-- | clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst | 137 |
2 files changed, 138 insertions, 0 deletions
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 8af60124e02..14ea9e96e7b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -67,6 +67,7 @@ Clang-Tidy Checks misc-non-copyable-objects misc-pointer-and-integral-operation misc-sizeof-container + misc-sizeof-expression misc-static-assert misc-string-integer-assignment misc-string-literal-with-embedded-nul diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst new file mode 100644 index 00000000000..4b92ae7ce01 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst @@ -0,0 +1,137 @@ +.. title:: clang-tidy - misc-sizeof-expression + +misc-sizeof-expression +====================== + +The check finds usages of ``sizeof`` expressions which are most likely errors. + +The ``sizeof`` operator yields the size (in bytes) of its operand, which may be +an expression or the parenthesized name of a type. Misuse of this operator may +be leading to errors and possible software vulnerabilities. + + +Suspicious usage of 'sizeof(K)' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +A common mistake is to query the ``sizeof`` of an integer literal. This is +equivalent to query the size of its type (probably ``int``). The intent of the +programmer was probably to simply get the integer and not its size. + +.. code:: c++ + + #define BUFLEN 42 + char buf[BUFLEN]; + memset(buf, 0, sizeof(BUFLEN)); // sizeof(42) ==> sizeof(int) + + +Suspicious usage of 'sizeof(this)' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +The ``this`` keyword is evaluated to a pointer to an object of a given type. +The expression ``sizeof(this)`` is returning the size of a pointer. The +programmer most likely wanted the size of the object and not the size of the +pointer. + +.. code:: c++ + + class Point { + [...] + size_t size() { return sizeof(this); } // should probably be sizeof(*this) + [...] + }; + + +Suspicious usage of 'sizeof(char*)' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +There is a subtle difference between declaring a string literal with +``char* A = ""`` and ``char A[] = ""``. The first case has the type ``char*`` +instead of the aggregate type ``char[]``. Using ``sizeof`` on an object declared +with ``char*`` type is returning the size of a pointer instead of the number of +characters (bytes) in the string literal. + +.. code:: c++ + + const char* kMessage = "Hello World!"; // const char kMessage[] = "..."; + void getMessage(char* buf) { + memcpy(buf, kMessage, sizeof(kMessage)); // sizeof(char*) + } + + +Suspicious usage of 'sizeof(A*)' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +A common mistake is to compute the size of a pointer instead of its pointee. +These cases may occur because of explicit cast or implicit conversion. + +.. code:: c++ + + int A[10]; + memset(A, 0, sizeof(A + 0)); + + struct Point point; + memset(point, 0, sizeof(&point)); + + +Suspicious usage of 'sizeof(...)/sizeof(...)' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Dividing ``sizeof`` expressions is typically used to retrieve the number of +elements of an aggregate. This check warns on incompatible or suspicious cases. + +In the following example, the entity has 10-bytes and is incompatible with the +type ``int`` which has 4 bytes. + +.. code:: c++ + + char buf[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; // sizeof(buf) => 10 + void getMessage(char* dst) { + memcpy(dst, buf, sizeof(buf) / sizeof(int)); // sizeof(int) => 4 [incompatible sizes] + } + + +In the following example, the expression ``sizeof(Values)`` is returning the +size of ``char*``. One can easily be fooled by its declaration, but in parameter +declaration the size '10' is ignored and the function is receiving a ``char*``. + +.. code:: c++ + + char OrderedValues[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; + return CompareArray(char Values[10]) { + return memcmp(OrderedValues, Values, sizeof(Values)) == 0; // sizeof(Values) ==> sizeof(char*) [implicit cast to char*] + } + + +Suspicious 'sizeof' by 'sizeof' expression +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Multiplying ``sizeof`` expressions typically makes no sense and is probably a +logic error. In the following example, the programmer used ``*`` instead of +``/``. + +.. code:: c++ + + const char kMessage[] = "Hello World!"; + void getMessage(char* buf) { + memcpy(buf, kMessage, sizeof(kMessage) * sizeof(char)); // sizeof(kMessage) / sizeof(char) + } + + +This check may trigger on code using the arraysize macro. The following code is +working correctly but should be simplified by using only the ``sizeof`` +operator. + +.. code:: c++ + + extern Object objects[100]; + void InitializeObjects() { + memset(objects, 0, arraysize(objects) * sizeof(Object)); // sizeof(objects) + } + + +Suspicious usage of 'sizeof(sizeof(...))' +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Getting the ``sizeof`` of a ``sizeof`` makes no sense and is typically an error +hidden through macros. + +.. code:: c++ + + #define INT_SZ sizeof(int) + int buf[] = { 42 }; + void getInt(int* dst) { + memcpy(dst, buf, sizeof(INT_SZ)); // sizeof(sizeof(int)) is suspicious. + } |