Fix #15027 (man/checkers: document remaining checkers) [skip ci]

Author: dmcppcheck

man/checkers/AssignmentAddressToInteger.md

@@ -0,0 +1,56 @@
+# AssignmentAddressToInteger and AssignmentIntegerToAddress
+
+**Message**: Assigning a pointer to an integer is not portable.<br/>
+**Category**: Portability<br/>
+**Severity**: Portability<br/>
+**Language**: C/C++
+
+## Description
+
+Code silently narrows a pointer (address) down to a plain integer type, or the other way around, via a
+plain assignment. On platforms where `sizeof(void*) != sizeof(int)` (most notably 64-bit platforms,
+where a pointer is 8 bytes and `int` is usually still 4 bytes) this loses information: part of the
+address is silently discarded.
+
+- `AssignmentAddressToInteger`: a pointer value is assigned to a plain integer variable, for example
+  `int i = p;`.
+- `AssignmentIntegerToAddress`: a plain integer value is assigned to a pointer variable, for example
+  `int *p = i;`.
+
+This checks `char`/`short`/`int` variables (not `long`/`long long`, and not `bool`, which is a common,
+intentional null-check idiom rather than a truncation bug). This checker only runs when the
+`portability` severity is enabled.
+
+## Motivation
+
+Storing an address in a type that is narrower than a pointer is not portable: it works by accident on
+platforms where the two types happen to be the same width, and silently truncates the address (or
+sign-extends a small integer into a bogus address) on platforms where they are not, most notably when
+porting 32-bit code to 64-bit.
+
+## How to fix
+
+Use a pointer type, or an integer type explicitly meant to hold a pointer (`intptr_t`/`uintptr_t` from
+`<cstdint>`), instead of a plain `int`/`char`/etc.
+
+Before:
+```cpp
+int foo(int *p) {
+    int a = p; // <- AssignmentAddressToInteger
+    return a;
+}
+```
+
+After:
+```cpp
+#include <cstdint>
+intptr_t foo(int *p) {
+    intptr_t a = reinterpret_cast<intptr_t>(p);
+    return a;
+}
+```
+
+## Related checkers
+
+- [CastAddressToIntegerAtReturn.md](CastAddressToIntegerAtReturn.md) - the same idea, but for a
+  function `return` rather than a plain assignment.

man/checkers/CastAddressToIntegerAtReturn.md

@@ -0,0 +1,55 @@
+# CastAddressToIntegerAtReturn and CastIntegerToAddressAtReturn
+
+**Message**: Returning an address value in a function with integer return type is not portable.<br/>
+**Category**: Portability<br/>
+**Severity**: Portability<br/>
+**Language**: C/C++
+
+## Description
+
+A function silently narrows a pointer (address) down to a plain integer return type, or the other way
+around. On platforms where `sizeof(void*) != sizeof(int)` (most notably 64-bit platforms, where a
+pointer is 8 bytes and `int` is usually still 4 bytes) this loses information: part of the address is
+silently discarded.
+
+- `CastAddressToIntegerAtReturn`: a function with an integer return type returns a pointer value, for
+  example `int foo(char *p) { return p; }`.
+- `CastIntegerToAddressAtReturn`: a function with a pointer return type returns a plain integer value,
+  for example `void* foo(int i) { return i; }`.
+
+This checks `char`/`short`/`int` (not `long`/`long long`, and not `bool`, which is a common,
+intentional idiom rather than a truncation bug), and only when analyzing for a 64-bit target - on a
+32-bit target a pointer and an `int` are the same width, so returning one as the other isn't a
+portability problem there. This checker only runs when the `portability` severity is enabled.
+
+## Motivation
+
+Storing an address in a type that is narrower than a pointer is not portable: it works by accident on
+platforms where the two types happen to be the same width, and silently truncates the address (or
+sign-extends a small integer into a bogus address) on platforms where they are not, most notably when
+porting 32-bit code to 64-bit.
+
+## How to fix
+
+Use a pointer type, or an integer type explicitly meant to hold a pointer (`intptr_t`/`uintptr_t` from
+`<cstdint>`), instead of a plain `int`/`char`/etc.
+
+Before:
+```cpp
+void* foo(int i) {
+    return i; // <- CastIntegerToAddressAtReturn
+}
+```
+
+After:
+```cpp
+#include <cstdint>
+void* foo(intptr_t i) {
+    return reinterpret_cast<void*>(i);
+}
+```
+
+## Related checkers
+
+- [AssignmentAddressToInteger.md](AssignmentAddressToInteger.md) - the same idea, but for a plain
+  assignment rather than a function `return`.

man/checkers/IOWithoutPositioning.md

@@ -0,0 +1,57 @@
+# IOWithoutPositioning
+
+**Message**: Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush in between result in undefined behaviour.<br/>
+**Category**: Undefined Behaviour<br/>
+**Severity**: Error<br/>
+**Language**: C/C++
+
+## Description
+
+A read is immediately followed by a write (or vice versa) on a file opened for both, with no
+`fseek()`/`fsetpos()`/`rewind()`/`fflush()` in between - the C standard says this is undefined
+behaviour.
+
+## Motivation
+
+The C standard requires a positioning call (or an `fflush()`) between a read and a following write (or
+vice versa) on the same read/write stream. Skipping it is undefined behaviour, even though many
+implementations happen to do something predictable with it. cppcheck only follows a local `FILE*`
+variable through straight-line code in the function that opened it; a global or member file handle, or
+passing the handle to another function, is enough uncertainty that it stops checking rather than guess,
+so this only catches the mismatches it can actually prove.
+
+## How to fix
+
+Before:
+```cpp
+#include <cstdio>
+void f() {
+    FILE *fp = fopen("a.txt", "r+");
+    if (!fp) return;
+    char buf[10];
+    fread(buf, 1, 10, fp);
+    fwrite(buf, 1, 10, fp); // <- no seek/rewind/fflush since the read above
+    fclose(fp);
+}
+```
+
+After:
+```cpp
+#include <cstdio>
+void f() {
+    FILE *fp = fopen("a.txt", "r+");
+    if (!fp) return;
+    char buf[10];
+    fread(buf, 1, 10, fp);
+    fseek(fp, 0, SEEK_CUR);
+    fwrite(buf, 1, 10, fp);
+    fclose(fp);
+}
+```
+
+## Related checkers
+
+- [useClosedFile.md](useClosedFile.md), [readWriteOnlyFile.md](readWriteOnlyFile.md),
+  [writeReadOnlyFile.md](writeReadOnlyFile.md), [seekOnAppendedFile.md](seekOnAppendedFile.md),
+  [incompatibleFileOpen.md](incompatibleFileOpen.md) - other checks that follow the same `FILE*` through
+  a function.

man/checkers/StlMissingComparison.md

@@ -0,0 +1,42 @@
+# StlMissingComparison
+
+**Message**: Missing bounds check for extra iterator increment in loop.<br/>
+**Category**: Undefined Behaviour<br/>
+**Severity**: Warning<br/>
+**Language**: C++
+
+## Description
+
+Inside a loop, the iterator is incremented a second time (in addition to the loop's own increment)
+without any bounds check in between, risking incrementing it past `end()`.
+
+## Motivation
+
+An iterator that's advanced twice per iteration without checking for `end()` in between can walk right
+past the end of the container - dereferencing it afterwards, or even just comparing it again, is then
+undefined behaviour.
+
+## How to fix
+
+Before:
+```cpp
+#include <set>
+void f(std::set<int> &ints, bool a) {
+    for (std::set<int>::iterator it = ints.begin(); it != ints.end(); ++it) {
+        if (a) {
+            it++; // <- StlMissingComparison: might increment 'it' past end()
+        }
+    }
+}
+```
+
+After: don't increment the iterator a second time inside the loop body.
+```cpp
+#include <set>
+void f(std::set<int> &ints, bool a) {
+    for (std::set<int>::iterator it = ints.begin(); it != ints.end(); ++it) {
+        if (a) {
+        }
+    }
+}
+```

man/checkers/UnionZeroInit.md

@@ -0,0 +1,43 @@
+# UnionZeroInit
+
+**Message**: You are using memset() to initialize a union that also contains a member with a bigger size.<br/>
+**Category**: Portability<br/>
+**Severity**: Portability<br/>
+**Language**: C/C++
+
+## Description
+
+A union is zero-initialized (`= {0}` or `= {}`), but its largest member isn't declared first - only the
+first member is guaranteed to be fully written by that initializer, so the rest of the union's storage
+(beyond the first member's size) may not actually end up zeroed.
+
+## Motivation
+
+An aggregate initializer for a union only initializes the first named member. If a smaller member is
+listed first, the initializer only guarantees that member's bytes are zeroed - the remaining bytes,
+which are only reachable through a later, larger member, are left with whatever was already in memory.
+Code that expects the whole union to be zero can then read uninitialized bytes through the larger
+member.
+
+## How to fix
+
+Declare the union's largest member first, so a `{0}`/`{}` initializer zeroes its entire storage.
+
+Before:
+```cpp
+void foo() {
+    union { char c; int i; } bad0 = {0}; // <- 'i' (the larger member) isn't first
+}
+```
+
+After:
+```cpp
+void foo() {
+    union { int i; char c; } good0 = {0};
+}
+```
+
+## Related checkers
+
+- [overlappingWriteUnion.md](overlappingWriteUnion.md) - a different union-related pitfall, about
+  reading and writing two overlapping members in the same expression.

man/checkers/accessMoved.md

@@ -0,0 +1,70 @@
+# accessMoved and accessForwarded
+
+**Message**: Access of moved variable 'v'.<br/>
+**Category**: Correctness<br/>
+**Severity**: Warning<br/>
+**Language**: C++ only
+
+## Description
+
+A variable is read after it has been passed to `std::move()` (`accessMoved`) or `std::forward()`
+(`accessForwarded`) - by convention, once a value has been moved/forwarded from, its contents are
+unspecified and shouldn't be relied on (except to reset or destroy it).
+
+## Motivation
+
+`std::move()`/`std::forward()` don't themselves do anything except change how the compiler treats the
+expression - the actual "move" happens in whatever constructor or assignment operator the moved-from
+value is subsequently passed into, and its effect on the source object is entirely up to that type. By
+convention a moved-from object is left in a valid but unspecified state, so any code that reads its
+value afterwards (rather than just reassigning or destroying it) is relying on something the language
+doesn't guarantee.
+
+## How to fix
+
+Before:
+```cpp
+#include <utility>
+struct A {};
+void g(A a);
+void f() {
+    A a;
+    g(std::move(a));
+    g(std::move(a)); // <- accessMoved: 'a' was already moved from above
+}
+```
+
+After:
+```cpp
+#include <utility>
+struct A {};
+void g(A a);
+void f() {
+    A a;
+    g(std::move(a));
+}
+```
+
+Before:
+```cpp
+#include <utility>
+template<typename T>
+void g(T&&);
+template<typename T>
+void f(T && t) {
+    g(std::forward<T>(t));
+    T s = t; // <- accessForwarded: 't' was already forwarded above
+}
+```
+
+After:
+```cpp
+#include <utility>
+template<typename T>
+void g(T&&);
+template<typename T>
+void f(T && t) {
+    T s = t;
+    g(std::forward<T>(t));
+}
+```

man/checkers/algorithmOutOfBounds.md

@@ -1,7 +1,7 @@
 # algorithmOutOfBounds
 
 **Message**: The algorithm 'std::copy' accesses 5 elements through the iterator 'v1.begin()' but only 3 elements are available.<br/>
-**Category**: Correctness<br/>
+**Category**: Undefined Behaviour<br/>
 **Severity**: Error<br/>
 **Language**: C++
 
@@ -11,8 +11,8 @@ Many STL algorithms take an iterator that denotes the beginning of a second rang
 assume that this range is large enough. If it is not, the algorithm writes or reads past the end of the container,
 which is undefined behavior.
 
-This checker uses the ValueFlow analysis to compare the number of elements an algorithm accesses with the number of
-elements that are actually available through the iterator, and warns when the access is out of bounds. Three groups
+This checker compares the number of elements an algorithm accesses with the number of elements that
+are actually available through the iterator, and warns when the access is out of bounds. Three groups
 of algorithms are checked:
 
 - Algorithms that access exactly `last1 - first1` elements through the other iterator: `std::copy`, `std::move`,

man/checkers/allocaCalled.md

@@ -0,0 +1,45 @@
+# allocaCalled
+
+**Message**: Obsolete function 'alloca' called. In C99 and later it is recommended to use a variable length array instead.<br/>
+**Category**: Correctness<br/>
+**Severity**: Warning<br/>
+**Language**: C/C++
+
+## Description
+
+`alloca()` is called. `alloca()` allocates memory on the stack that is freed automatically when the
+calling function returns, but it has no standard error-handling convention (a too-large request can
+overflow the stack silently) and its lifetime rules are easy to get wrong (the memory becomes invalid
+the moment the calling function returns, even if a pointer to it is kept around).
+
+## Motivation
+
+Unlike `malloc()`, `alloca()` has no way to report failure - if the requested size is too large, the
+program's behaviour is undefined (typically a stack overflow) rather than a clean, checkable error. It's
+also easy to accidentally keep using the returned pointer after the function that called `alloca()` has
+returned, which is undefined behaviour.
+
+## How to fix
+
+Before:
+```cpp
+#include <alloca.h>
+void f(int n) {
+    char *buf = alloca(n); // <- obsolete, no error handling if 'n' is too large
+}
+```
+
+After (C99 and later):
+```cpp
+void f(int n) {
+    char buf[n]; // variable length array
+}
+```
+
+After (C++11 and later):
+```cpp
+#include <array>
+void f() {
+    std::array<char, 128> buf; // fixed-size, or use a dynamically allocated container
+}
+```