[clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. #66207

balazske · 2023-09-13T13:24:39Z

The checker is good enough to move out of alpha.

… alpha. This checker can be good enough to move out of alpha. I am not sure about the exact requirements, this review can be a place for discussion about what should be fixed (if any). Reviewed By: steakhal Differential Revision: https://reviews.llvm.org/D152436

llvmbot · 2023-09-13T13:26:04Z

@llvm/pr-subscribers-clang

Changes

The checker is good enough to move out of alpha. --

Patch is 48.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66207.diff

37 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/docs/analyzer/checkers.rst (+94-94)
(modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+22-21)
(modified) clang/test/Analysis/PR49642.c (+1-1)
(modified) clang/test/Analysis/analyzer-config.c (+2-2)
(modified) clang/test/Analysis/analyzer-enabled-checkers.c (+1)
(modified) clang/test/Analysis/conversion.c (+2-2)
(modified) clang/test/Analysis/errno-stdlibraryfunctions-notes.c (+2-2)
(modified) clang/test/Analysis/errno-stdlibraryfunctions.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-POSIX-lookup.c (+3-3)
(modified) clang/test/Analysis/std-c-library-functions-POSIX-socket-sockaddr.cpp (+3-3)
(modified) clang/test/Analysis/std-c-library-functions-POSIX.c (+6-6)
(modified) clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-arg-constraints-tracking-notes.c (+1-1)
(modified) clang/test/Analysis/std-c-library-functions-arg-constraints.c (+4-4)
(modified) clang/test/Analysis/std-c-library-functions-arg-constraints.cpp (+1-1)
(modified) clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c (+5-5)
(modified) clang/test/Analysis/std-c-library-functions-arg-weakdeps.c (+5-5)
(modified) clang/test/Analysis/std-c-library-functions-eof.c (+5-5)
(modified) clang/test/Analysis/std-c-library-functions-inlined.c (+5-5)
(modified) clang/test/Analysis/std-c-library-functions-lookup.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-lookup.cpp (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-path-notes.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-restrict.c (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-restrict.cpp (+2-2)
(modified) clang/test/Analysis/std-c-library-functions-vs-stream-checker.c (+4-4)
(modified) clang/test/Analysis/std-c-library-functions.c (+6-6)
(modified) clang/test/Analysis/std-c-library-functions.cpp (+1-1)
(modified) clang/test/Analysis/std-c-library-posix-crash.c (+2-2)
(modified) clang/test/Analysis/stream-errno-note.c (+2-2)
(modified) clang/test/Analysis/stream-errno.c (+2-2)
(modified) clang/test/Analysis/stream-noopen.c (+4-4)
(modified) clang/test/Analysis/stream-note.c (+2-2)
(modified) clang/test/Analysis/stream-stdlibraryfunctionargs.c (+5-5)
(modified) clang/test/Analysis/weak-dependencies.c (+1-1)

<pre>
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..dd10e707b2f561c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -408,6 +408,8 @@ Static Analyzer

Added a new checker core.BitwiseShift which reports situations where
bitwise shift operators produce undefined behavior (because some operand is
negative or too large).
+- Move checker alpha.unix.StdCLibraryFunctions out of the alpha package

to unix.StdCLibraryFunctions.

.. _release-notes-sanitizers:

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 54ea49e7426cc86..998a9e888f3a3b3 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1026,6 +1026,100 @@ Check for null pointers being passed as arguments to C string functions:
return strlen(0); // warn
}

+.. _unix-StdCLibraryFunctions:
+
+unix.StdCLibraryFunctions (C)
+"""""""""""""""""""""""""""""""""""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the int +isalnum(int ch) function the behavior is undefined if the value of ch is
+not representable as unsigned char and is not equal to EOF.
+
+.. code-block:: c
+

#define EOF -1
void test_alnum_concrete(int v) {
int ret = isalnum(256); // \
// warning: Function argument outside of allowed range
(void)ret;
}
void buffer_size_violation(FILE *file) {
enum { BUFFER_SIZE = 1024 };
wchar_t wbuf[BUFFER_SIZE];
const size_t size = sizeof(*wbuf); // 4
const size_t nitems = sizeof(wbuf); // 4096
// Below we receive a warning because the 3rd parameter should be the
// number of elements to read, not the size in bytes. This case is a known
// vulnerability described by the ARR38-C SEI-CERT rule.
fread(wbuf, size, nitems, file);
}

+You can think of this checker as defining restrictions (pre- and postconditions)
+on standard library functions. Preconditions are checked, and when they are
+violated, a warning is emitted. Post conditions are added to the analysis, e.g.
+that the return value must be no greater than 255.
+
+For example if an argument to a function must be in between 0 and 255, but the
+value of the argument is unknown, the analyzer will conservatively assume that
+it is in this interval. Similarly, if a function mustn't be called with a null
+pointer and the null value of the argument can not be proven, the analyzer will
+assume that it is non-null.
+
+These are the possible checks on the values passed as function arguments:

- The argument has an allowed range (or multiple ranges) of values. The checker
can detect if a passed value is outside of the allowed range and show the
actual and allowed values.
- The argument has pointer type and is not allowed to be null pointer. Many
(but not all) standard functions can produce undefined behavior if a null
pointer is passed, these cases can be detected by the checker.
- The argument is a pointer to a memory block and the minimal size of this
buffer is determined by another argument to the function, or by
multiplication of two arguments (like at function fread), or is a fixed
value (for example asctime_r requires at least a buffer of size 26). The
checker can detect if the buffer size is too small and in optimal case show
the size of the buffer and the values of the corresponding arguments.

+.. code-block:: c
+

int test_alnum_symbolic(int x) {
int ret = isalnum(x);
// after the call, ret is assumed to be in the range [-1, 255]
if (ret > 255) // impossible (infeasible branch)
```
 if (x == 0)
```

   return ret / x; // division by zero is not reported

return ret;
}

+Additionally to the argument and return value conditions, this checker also adds
+state of the value errno if applicable to the analysis. Many system
+functions set the errno value only if an error occurs (together with a
+specific return value of the function), otherwise it becomes undefined. This
+checker changes the analysis state to contain such information. This data is
+used by other checkers, for example :ref:alpha-unix-Errno.
+
+Limitations
+
+The checker can not always provide notes about the values of the arguments.
+Without this information it is hard to confirm if the constraint is indeed
+violated. The argument values are shown if they are known constants or the value
+is determined by previous (not too complicated) assumptions.
+
+The checker can produce false positives in cases such as if the program has
+invariants not known to the analyzer engine or the bug report path contains
+calls to unknown functions. In these cases the analyzer fails to detect the real
+range of the argument.
+
+Parameters
+
+The checker models functions (and emits diagnostics) from the C standard by
+default. The ModelPOSIX option enables modeling (and emit diagnostics) of
+additional functions that are defined in the POSIX standard. This option is
+disabled by default.
+
.. _osx-checkers:

osx
@@ -2651,100 +2745,6 @@ For a more detailed description of configuration options, please see the
alpha.unix
^^^^^^^^^^^

-.. _alpha-unix-StdCLibraryFunctions:

-alpha.unix.StdCLibraryFunctions (C)
-"""""""""""""""""""""""""""""""""""
-Check for calls of standard library functions that violate predefined argument
-constraints. For example, it is stated in the C standard that for the `int -isalnum(int ch)` function the behavior is undefined if the value of `ch` is
-not representable as unsigned char and is not equal to `EOF`.

-.. code-block:: c

#define EOF -1
void test_alnum_concrete(int v) {
int ret = isalnum(256); // \
// warning: Function argument outside of allowed range
(void)ret;
}
void buffer_size_violation(FILE *file) {
enum { BUFFER_SIZE = 1024 };
wchar_t wbuf[BUFFER_SIZE];
const size_t size = sizeof(*wbuf); // 4
const size_t nitems = sizeof(wbuf); // 4096
// Below we receive a warning because the 3rd parameter should be the
// number of elements to read, not the size in bytes. This case is a known
// vulnerability described by the ARR38-C SEI-CERT rule.
fread(wbuf, size, nitems, file);
}

-You can think of this checker as defining restrictions (pre- and postconditions)
-on standard library functions. Preconditions are checked, and when they are
-violated, a warning is emitted. Post conditions are added to the analysis, e.g.
-that the return value must be no greater than 255.

-For example if an argument to a function must be in between 0 and 255, but the
-value of the argument is unknown, the analyzer will conservatively assume that
-it is in this interval. Similarly, if a function mustn't be called with a null
-pointer and the null value of the argument can not be proven, the analyzer will
-assume that it is non-null.

-These are the possible checks on the values passed as function arguments:

- The argument has an allowed range (or multiple ranges) of values. The checker
can detect if a passed value is outside of the allowed range and show the
actual and allowed values.
- The argument has pointer type and is not allowed to be null pointer. Many
(but not all) standard functions can produce undefined behavior if a null
pointer is passed, these cases can be detected by the checker.
- The argument is a pointer to a memory block and the minimal size of this
buffer is determined by another argument to the function, or by
multiplication of two arguments (like at function fread), or is a fixed
value (for example asctime_r requires at least a buffer of size 26). The
checker can detect if the buffer size is too small and in optimal case show
the size of the buffer and the values of the corresponding arguments.

-.. code-block:: c

int test_alnum_symbolic(int x) {
int ret = isalnum(x);
// after the call, ret is assumed to be in the range [-1, 255]
if (ret > 255) // impossible (infeasible branch)
```
 if (x == 0)
```

   return ret / x; // division by zero is not reported

return ret;
}

-Additionally to the argument and return value conditions, this checker also adds
-state of the value `errno` if applicable to the analysis. Many system
-functions set the `errno` value only if an error occurs (together with a
-specific return value of the function), otherwise it becomes undefined. This
-checker changes the analysis state to contain such information. This data is
-used by other checkers, for example :ref:`alpha-unix-Errno`.

-Limitations

-The checker can not always provide notes about the values of the arguments.
-Without this information it is hard to confirm if the constraint is indeed
-violated. The argument values are shown if they are known constants or the value
-is determined by previous (not too complicated) assumptions.

-The checker can produce false positives in cases such as if the program has
-invariants not known to the analyzer engine or the bug report path contains
-calls to unknown functions. In these cases the analyzer fails to detect the real
-range of the argument.

-Parameters

-The checker models functions (and emits diagnostics) from the C standard by
-default. The `ModelPOSIX` option enables modeling (and emit diagnostics) of
-additional functions that are defined in the POSIX standard. This option is
-disabled by default.

.. _alpha-unix-BlockInCriticalSection:

alpha.unix.BlockInCriticalSection (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 65c1595eb6245dd..d4c1e0532f9c56d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -532,6 +532,27 @@ def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">,
Dependencies<[DynamicMemoryModeling]>,
Documentation<HasDocumentation>;

+def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,

HelpText<"Check for invalid arguments of C standard library functions, "

      &amp;quot;and apply relations between arguments and return value&amp;quot;&amp;gt;,

CheckerOptions<[
CmdLineOption<Boolean,

             &amp;quot;DisplayLoadedSummaries&amp;quot;,

             &amp;quot;If set to true, the checker displays the found summaries &amp;quot;

             &amp;quot;for the given translation unit.&amp;quot;,

             &amp;quot;false&amp;quot;,

```
             Released,
```
```
             Hide&amp;gt;,
```
CmdLineOption<Boolean,

             &amp;quot;ModelPOSIX&amp;quot;,

             &amp;quot;If set to true, the checker models additional functions &amp;quot;

             &amp;quot;from the POSIX standard.&amp;quot;,

             &amp;quot;false&amp;quot;,

```
             InAlpha&amp;gt;
```
]>,
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
Documentation<HasDocumentation>;

def VforkChecker : Checker<"Vfork">,
HelpText<"Check for proper usage of vfork">,
Documentation<HasDocumentation>;
@@ -574,27 +595,6 @@ def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
HelpText<"Check for calls to blocking functions inside a critical section">,
Documentation<HasDocumentation>;

-def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,

HelpText<"Check for invalid arguments of C standard library functions, "

      &amp;quot;and apply relations between arguments and return value&amp;quot;&amp;gt;,

CheckerOptions<[
CmdLineOption<Boolean,

             &amp;quot;DisplayLoadedSummaries&amp;quot;,

             &amp;quot;If set to true, the checker displays the found summaries &amp;quot;

             &amp;quot;for the given translation unit.&amp;quot;,

             &amp;quot;false&amp;quot;,

```
             Released,
```
```
             Hide&amp;gt;,
```
CmdLineOption<Boolean,

             &amp;quot;ModelPOSIX&amp;quot;,

             &amp;quot;If set to true, the checker models additional functions &amp;quot;

             &amp;quot;from the POSIX standard.&amp;quot;,

             &amp;quot;false&amp;quot;,

```
             InAlpha&amp;gt;
```
]>,
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
Documentation<HasDocumentation>;

} // end "alpha.unix"

//===----------------------------------------------------------------------===//
@@ -1622,6 +1622,7 @@ def DebugIteratorModeling : Checker<"DebugIteratorModeling">,
def StdCLibraryFunctionsTesterChecker : Checker<"StdCLibraryFunctionsTester">,
HelpText<"Add test functions to the summary map, so testing of individual "
"summary constituents becomes possible.">,

WeakDependencies<[StdCLibraryFunctionsChecker]>,
Documentation<NotDocumented>;

} // end "debug"
diff --git a/clang/test/Analysis/PR49642.c b/clang/test/Analysis/PR49642.c
index c21050fd4a5c878..78bbde79d83006d 100644
--- a/clang/test/Analysis/PR49642.c
+++ b/clang/test/Analysis/PR49642.c
@@ -1,6 +1,6 @@
// RUN: %clang_analyze_cc1 -Wno-implicit-function-declaration -Wno-implicit-int -w -verify %s
// RUN: -analyzer-checker=core
-// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctions
+// RUN: -analyzer-checker=unix.StdCLibraryFunctions

// expected-no-diagnostics

diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index d86ca5d19219c64..794ef8b9cc08680 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -13,8 +13,6 @@
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
-// CHECK-NEXT: alpha.unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
-// CHECK-NEXT: alpha.unix.StdCLibraryFunctions:ModelPOSIX = false
// CHECK-NEXT: apply-fixits = false
// CHECK-NEXT: assume-controlled-environment = false
// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
@@ -129,6 +127,8 @@
// CHECK-NEXT: track-conditions-debug = false
// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
+// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
+// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: verbose-report-filename = false
// CHECK-NEXT: widen-loops = false
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index ed8334b9e2db009..cf69a6b04c97922 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -47,6 +47,7 @@
// CHECK-NEXT: unix.Malloc
// CHECK-NEXT: unix.MallocSizeof
// CHECK-NEXT: unix.MismatchedDeallocator
+// CHECK-NEXT: unix.StdCLibraryFunctions
// CHECK-NEXT: unix.Vfork
// CHECK-NEXT: unix.cstring.BadSizeArg
// CHECK-NEXT: unix.cstring.NullArg
diff --git a/clang/test/Analysis/conversion.c b/clang/test/Analysis/conversion.c
index 0d2e005550b16c1..cafe9c37c240225 100644
--- a/clang/test/Analysis/conversion.c
+++ b/clang/test/Analysis/conversion.c
@@ -1,6 +1,6 @@
// RUN: %clang_analyze_cc1 %s
// RUN: -Wno-conversion -Wno-tautological-constant-compare
-// RUN: -analyzer-checker=core,apiModeling,alpha.unix.StdCLibraryFunctions,alpha.core.Conversion
+// RUN: -analyzer-checker=core,apiModeling,unix.StdCLibraryFunctions,alpha.core.Conversion
// RUN: -verify

unsigned char U8;
@@ -187,7 +187,7 @@ char dontwarn10(long long x) {
}

-// C library functions, handled via alpha.unix.StdCLibraryFunctions
+// C library functions, handled via unix.StdCLibraryFunctions

int isascii(int c);
void libraryFunction1(void) {
diff --git a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
index 991384cc373ef3a..c3fac58c46b3763 100644
--- a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
+++ b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
@@ -1,10 +1,10 @@
// RUN: %clang_analyze_cc1 -verify -analyzer-output text %s
// RUN: -analyzer-checker=core
// RUN: -analyzer-checker=debug.ExprInspection
-// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctions
+// RUN: -analyzer-checker=unix.StdCLibraryFunctions
// RUN: -analyzer-checker=apiModeling.Errno
// RUN: -analyzer-checker=alpha.unix.Errno
-// RUN: -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true
+// RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true

#include "Inputs/errno_var.h"

diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c
index a3b42f4425c3525..fce5e5d6b0a4710 100644
--- a/clang/test/Analys...

NagyDonat

(I started to nitpick the documentation text before I realized that it's just old content moved in from elsewhere. Probably it's better to handle these in a separate commit.)

NagyDonat · 2023-09-14T09:40:57Z

clang/docs/analyzer/checkers.rst

+You can think of this checker as defining restrictions (pre- and postconditions)
+on standard library functions. Preconditions are checked, and when they are
+violated, a warning is emitted. Post conditions are added to the analysis, e.g.
+that the return value must be no greater than 255.


"the return value" of what? I think a function name is missing here.

NagyDonat · 2023-09-14T09:45:46Z

clang/docs/analyzer/checkers.rst

+pointer and the null value of the argument can not be proven, the analyzer will
+assume that it is non-null.


Suggested change

pointer and the null value of the argument can not be proven, the analyzer will

assume that it is non-null.

pointer and the analyzer cannot prove that it is null, then it will assume that

it is non-null.

"can not be proven" is stronger than "the analyzer cannot prove it"

NagyDonat · 2023-09-22T10:34:27Z

I tested this commit on several open-source projects, comparing it and its parent with a configuration that enables the non-alpha checkers (so StdCLibraryFunctions becomes enabled when this commit moves it out of alpha).

The results show that this checker doesn't produce random noise and can provide some useful results:

Project	New reports	Lost reports	Changes
memcached	New reports	Lost reports	no effect
tmux	New reports	Lost reports	no effect
twin	New reports	Lost reports	no effect
vim	New reports	Lost reports	no effect
openssl	New reports	Lost reports	no effect
sqlite	New reports	Lost reports	no effect
ffmpeg	New reports	Lost reports	no effect
postgres	New reports	Lost reports	5 new TPs [1]
tinyxml2	New reports	Lost reports	no effect
libwebm	New reports	Lost reports	no effect
xerces	New reports	Lost reports	no effect
bitcoin	New reports	Lost reports	no effect
protobuf	New reports	Lost reports	no effect

[1] One unix.StdCLibraryFunctions report and four very similar TOCTOU bugs reported by core.NonNullParamChecker (one example). These are all real issues, although it'd be very difficult to trigger them in practice. Note that we're testing stable versions of open-source projects, so it's not surprising that we don't see serious issues.

NagyDonat

Based on these clean test results I'd say that it's safe to move this checker out of alpha.

@haoNoQ @steakhal Do you have any objections?

steakhal

I didn't spent much time on this, but I think it should be good.
Please check the docs with Grammarly to catch mistakes.
Also, please generate the HTML for the rst to verify how it looks.

I'm not sure if the release docs mentions this, but it definitely should. Make sure that's the case.

I approve this, assuming that these wrinkles are ironed.

steakhal · 2023-09-29T16:54:30Z

clang/docs/analyzer/checkers.rst

+.. _unix-StdCLibraryFunctions:
+
+unix.StdCLibraryFunctions (C)
+"""""""""""""""""""""""""""""""""""


Align this with the section title.

steakhal · 2023-09-29T16:55:23Z

clang/docs/analyzer/checkers.rst

+"""""""""""""""""""""""""""""""""""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is


The grammar feels odd here.

steakhal · 2023-09-29T16:57:58Z

clang/docs/analyzer/checkers.rst

+
+You can think of this checker as defining restrictions (pre- and postconditions)
+on standard library functions. Preconditions are checked, and when they are
+violated, a warning is emitted. Post conditions are added to the analysis, e.g.


Here "post condition" is spelled as separate words, while previously there was a hyphen between the words.

steakhal · 2023-09-29T16:58:48Z

clang/docs/analyzer/checkers.rst

+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.


Char and unsigned are not escaped like we usually do for code.

steakhal · 2023-09-29T17:03:25Z

clang/docs/analyzer/checkers.rst

@@ -2651,100 +2745,6 @@ For a more detailed description of configuration options, please see the
 alpha.unix
 ^^^^^^^^^^^


As we are here, could you align this with its section title as well?

steakhal · 2023-09-29T17:04:13Z

clang/docs/analyzer/checkers.rst

+default. The ``ModelPOSIX`` option enables modeling (and emit diagnostics) of
+additional functions that are defined in the POSIX standard. This option is
+disabled by default.
+


Do you think we should have an exhaustive list of the modeled functions here, or that wouldn't be useful?

If I remember correctly the list of functions was already rejected by reviewers (because maintenance problems). Otherwise I think it is good to have an exact list of modeled functions.

steakhal · 2023-09-29T17:07:47Z

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+           "and apply relations between arguments and return value">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "DisplayLoadedSummaries",


Could you confirm that this option is not advertised in the user facing docs.

balazske · 2023-10-02T15:58:12Z

@haoNoQ Could you check if this change is OK to merge?

balazske requested review from a team as code owners September 13, 2023 13:24

llvmbot added clang Clang issues not falling into any other category clang:static analyzer clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html labels Sep 13, 2023

balazske requested review from NagyDonat and haoNoQ September 13, 2023 15:53

NagyDonat reviewed Sep 19, 2023

View reviewed changes

NagyDonat approved these changes Sep 22, 2023

View reviewed changes

NagyDonat requested a review from steakhal September 22, 2023 10:40

balazske removed the clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html label Sep 25, 2023

NagyDonat mentioned this pull request Sep 29, 2023

[analyzer] Add std::variant checker #66481

Merged

steakhal approved these changes Sep 29, 2023

View reviewed changes

Fixed documentation problems.

846092a

llvmbot added the clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html label Oct 2, 2023

balazske closed this Oct 16, 2023

balazske reopened this Oct 16, 2023

balazske merged commit c202a17 into llvm:main Oct 16, 2023

madhur13490 mentioned this pull request Oct 20, 2023

Revert commit ba8565fbcb975e2d067ce3ae5a7dbaae4953edd3 madhur13490/llvm-project#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. #66207

[clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. #66207

balazske commented Sep 13, 2023

llvmbot commented Sep 13, 2023

-.. _alpha-unix-StdCLibraryFunctions:

-.. code-block:: c

-You can think of this checker as defining restrictions (pre- and postconditions)
-on standard library functions. Preconditions are checked, and when they are
-violated, a warning is emitted. Post conditions are added to the analysis, e.g.
-that the return value must be no greater than 255.

-.. code-block:: c

-Limitations

-The checker can not always provide notes about the values of the arguments.
-Without this information it is hard to confirm if the constraint is indeed
-violated. The argument values are shown if they are known constants or the value
-is determined by previous (not too complicated) assumptions.

-The checker can produce false positives in cases such as if the program has
-invariants not known to the analyzer engine or the bug report path contains
-calls to unknown functions. In these cases the analyzer fails to detect the real
-range of the argument.

-Parameters

-The checker models functions (and emits diagnostics) from the C standard by
-default. The `ModelPOSIX` option enables modeling (and emit diagnostics) of
-additional functions that are defined in the POSIX standard. This option is
-disabled by default.

NagyDonat left a comment

NagyDonat Sep 14, 2023

NagyDonat Sep 14, 2023

NagyDonat commented Sep 22, 2023

NagyDonat left a comment

steakhal left a comment

steakhal Sep 29, 2023

steakhal Sep 29, 2023

steakhal Sep 29, 2023

steakhal Sep 29, 2023

steakhal Sep 29, 2023

steakhal Sep 29, 2023

balazske Oct 2, 2023

steakhal Sep 29, 2023

balazske commented Oct 2, 2023

		pointer and the null value of the argument can not be proven, the analyzer will
		assume that it is non-null.

		@@ -2651,100 +2745,6 @@ For a more detailed description of configuration options, please see the
		alpha.unix
		^^^^^^^^^^^

[clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. #66207

[clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha. #66207

Conversation

balazske commented Sep 13, 2023

llvmbot commented Sep 13, 2023

-.. _alpha-unix-StdCLibraryFunctions:

-.. code-block:: c

-You can think of this checker as defining restrictions (pre- and postconditions) -on standard library functions. Preconditions are checked, and when they are -violated, a warning is emitted. Post conditions are added to the analysis, e.g. -that the return value must be no greater than 255.

-.. code-block:: c

-Limitations

-The checker can not always provide notes about the values of the arguments. -Without this information it is hard to confirm if the constraint is indeed -violated. The argument values are shown if they are known constants or the value -is determined by previous (not too complicated) assumptions.

-The checker can produce false positives in cases such as if the program has -invariants not known to the analyzer engine or the bug report path contains -calls to unknown functions. In these cases the analyzer fails to detect the real -range of the argument.

-Parameters

-The checker models functions (and emits diagnostics) from the C standard by -default. The ModelPOSIX option enables modeling (and emit diagnostics) of -additional functions that are defined in the POSIX standard. This option is -disabled by default.

NagyDonat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NagyDonat commented Sep 22, 2023

NagyDonat left a comment

Choose a reason for hiding this comment

steakhal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balazske commented Oct 2, 2023

-You can think of this checker as defining restrictions (pre- and postconditions)
-on standard library functions. Preconditions are checked, and when they are
-violated, a warning is emitted. Post conditions are added to the analysis, e.g.
-that the return value must be no greater than 255.

-The checker can not always provide notes about the values of the arguments.
-Without this information it is hard to confirm if the constraint is indeed
-violated. The argument values are shown if they are known constants or the value
-is determined by previous (not too complicated) assumptions.

-The checker can produce false positives in cases such as if the program has
-invariants not known to the analyzer engine or the bug report path contains
-calls to unknown functions. In these cases the analyzer fails to detect the real
-range of the argument.

-The checker models functions (and emits diagnostics) from the C standard by
-default. The `ModelPOSIX` option enables modeling (and emit diagnostics) of
-additional functions that are defined in the POSIX standard. This option is
-disabled by default.