[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/NagyDonat approved this pull request. LGTM, I think that it isn't necessary to rerun the tests. I have one very minor suggestion to slightly improve the documentation. https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
@@ -1299,10 +1299,21 @@ 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. +The ``ModelPOSIX`` option controls if functions from the POSIX standard are +recognized by the checker. If ``true``, a big amount of POSIX functions is +modeled according to the +`POSIX standard`_. This +includes ranges of parameters and possible return values. Furthermore the +behavior related to ``errno`` in the POSIX case is often that ``errno`` is set +only if a function call fails, and it becomes undefined after a successful +function call. +If ``false``, functions are modeled according to the C99 language standard. +This includes far less functions than the POSIX case. It is possible that the +same functions are modeled differently in the two cases because differences in +the standards. The C standard specifies less aspects of the functions, for +example exact ``errno`` behavior is often unspecified (and not modeled by the +checker). +Default value of the option is ``true``. NagyDonat wrote: ```suggestion The ``ModelPOSIX`` option controls if functions from the POSIX standard are recognized by the checker. With ``ModelPOSIX=true``, lots of POSIX functions are modeled according to the `POSIX standard`_. This includes ranges of parameters and possible return values. Furthermore the behavior related to ``errno`` in the POSIX case is often that ``errno`` is set only if a function call fails, and it becomes undefined after a successful function call. With ``ModelPOSIX=false``, this checker follows the C99 language standard and only models the functions that are described there. It is possible that the same functions are modeled differently in the two cases because differences in the standards. The C standard specifies less aspects of the functions, for example exact ``errno`` behavior is often unspecified (and not modeled by the checker). Default value of the option is ``true``. ``` https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/80457
From 1f65abda712efce624c01ec99675c8261a8e6cea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Wed, 31 Jan 2024 17:40:21 +0100
Subject: [PATCH 1/2] {clang][analyzer] Change default value of checker option
in unix.StdCLibraryFunctions.
Default value of checker option `ModelPOSIX` is changed to `true`.
Documentation is updated.
---
clang/docs/analyzer/checkers.rst | 19 +++
.../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +-
clang/test/Analysis/analyzer-config.c | 2 +-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index bb637cf1b8007b..24522e56501e54 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1299,10 +1299,21 @@ 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.
+The ``ModelPOSIX`` option controls if functions from the POSIX standard are
+recognized by the checker. If ``true``, a big amount of POSIX functions is
+modeled according to the
+`POSIX standard`_. This
+includes ranges of parameters and possible return values. Furthermore the
+behavior related to ``errno`` in the POSIX case is often that ``errno`` is set
+only if a function call fails, and it becomes undefined after a successful
+function call.
+If ``false``, functions are modeled according to the C99 language standard.
+This includes far less functions than the POSIX case. It is possible that the
+same functions are modeled differently in the two cases because differences in
+the standards. The C standard specifies less aspects of the functions, for
+example exact ``errno`` behavior is often unspecified (and not modeled by the
+checker).
+Default value of the option is ``true``.
.. _osx-checkers:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index e7774e5a9392d2..a224b81c33a624 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -578,7 +578,7 @@ def StdCLibraryFunctionsChecker :
Checker<"StdCLibraryFunctions">,
"ModelPOSIX",
"If set to true, the checker models additional functions "
"from the POSIX standard.",
- "false",
+ "true",
InAlpha>
]>,
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
diff --git a/clang/test/Analysis/analyzer-config.c
b/clang/test/Analysis/analyzer-config.c
index 373017f4b18bfc..2167a2b32f5962 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -129,7 +129,7 @@
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
// CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
-// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
+// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = true
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: verbose-report-filename = false
// CHECK-NEXT: widen-loops = false
From 136b4c639dca08349e958f141620afea874924d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Thu, 29 Feb 2024 17:18:37 +0100
Subject: [PATCH 2/2] changed the documentation
---
clang/docs/analyzer/checkers.rst | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 24522e56501e54..9ede1657dcc356 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1300,19 +1300,21 @@ range of the argument.
**Parameters**
The ``ModelPOSIX`` option controls if functions from the POSIX standard are
-recognized by the checker. If ``true``, a big amount of POSIX functions is
-modeled according to the
-`POSIX standard`_. This
-includes ranges of parameters and possible return values. Furthermore the
-behavior related to ``errno`` in the POSIX case is often that ``errno`` is set
-only if a function call fails, and it becomes undefined after a successful
-function call.
-If ``false``, functions are modeled according to the C99 language standard.
-This includes far less functions than the POSIX case. It is possible that the
+recognized by the checker.
+
+With ``ModelPOSIX=true``, many POSIX functions are modeled according to the
+`POSIX standard`_. This includes ranges of parameters and possible return
+values. Furthermore the behavior related to ``errno`` in the POSIX case is
+often that ``errno`` is set on
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/balazske closed https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
balazske wrote: Behavior of `fileno` is already changed in #81842. I was thinking about that > separate long-term solution in last comment that it is already existing functionality (in StreamChecker and other invalid pointer checkers). Should we run again the checks (only modeling of `fileno` was changed), or is this change acceptable now? https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/80457
From 1f65abda712efce624c01ec99675c8261a8e6cea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Wed, 31 Jan 2024 17:40:21 +0100
Subject: [PATCH] {clang][analyzer] Change default value of checker option in
unix.StdCLibraryFunctions.
Default value of checker option `ModelPOSIX` is changed to `true`.
Documentation is updated.
---
clang/docs/analyzer/checkers.rst | 19 +++
.../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +-
clang/test/Analysis/analyzer-config.c | 2 +-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index bb637cf1b8007..24522e56501e5 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1299,10 +1299,21 @@ 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.
+The ``ModelPOSIX`` option controls if functions from the POSIX standard are
+recognized by the checker. If ``true``, a big amount of POSIX functions is
+modeled according to the
+`POSIX standard`_. This
+includes ranges of parameters and possible return values. Furthermore the
+behavior related to ``errno`` in the POSIX case is often that ``errno`` is set
+only if a function call fails, and it becomes undefined after a successful
+function call.
+If ``false``, functions are modeled according to the C99 language standard.
+This includes far less functions than the POSIX case. It is possible that the
+same functions are modeled differently in the two cases because differences in
+the standards. The C standard specifies less aspects of the functions, for
+example exact ``errno`` behavior is often unspecified (and not modeled by the
+checker).
+Default value of the option is ``true``.
.. _osx-checkers:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index e7774e5a9392d..a224b81c33a62 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -578,7 +578,7 @@ def StdCLibraryFunctionsChecker :
Checker<"StdCLibraryFunctions">,
"ModelPOSIX",
"If set to true, the checker models additional functions "
"from the POSIX standard.",
- "false",
+ "true",
InAlpha>
]>,
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
diff --git a/clang/test/Analysis/analyzer-config.c
b/clang/test/Analysis/analyzer-config.c
index 373017f4b18bf..2167a2b32f596 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -129,7 +129,7 @@
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
// CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
-// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
+// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = true
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: verbose-report-filename = false
// CHECK-NEXT: widen-loops = false
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
balazske wrote: The change was evaluated on the following projects. "Lost reports" shows results that disappear if the `ModelPOSIX` option is changed to true. "New reports" shows the new results. Many of the new results come from the large number of modeled functions. The lost reports are more interesting (some are at project postgres), probably the analysis changes because preconditions of functions are applied (if the option is turned on). | Project | Lost Reports | New Reports | |-|-|--| | memcached | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_modelposix_defaulton&newcheck=memcached_1.6.8_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_modelposix_defaulton&newcheck=memcached_1.6.8_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | tmux | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_modelposix_defaulton&newcheck=tmux_2.6_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_modelposix_defaulton&newcheck=tmux_2.6_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | curl | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | twin | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_modelposix_defaulton&newcheck=twin_v0.8.1_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_modelposix_defaulton&newcheck=twin_v0.8.1_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | vim | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_modelposix_defaulton&newcheck=vim_v8.2.1920_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_modelposix_defaulton&newcheck=vim_v8.2.1920_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | openssl | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_modelposix_defaulton&newcheck=openssl_openssl-3.0.0-alpha7_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_modelposix_defaulton&newcheck=openssl_openssl-3.0.0-alpha7_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | sqlite | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_modelposix_defaulton&newcheck=sqlite_version-3.33.0_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_modelposix_defaulton&newcheck=sqlite_version-3.33.0_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | ffmpeg | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_modelposix_defaulton&newcheck=ffmpeg_n4.3.1_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_modelposix_defaulton&newcheck=ffmpeg_n4.3.1_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | postgres | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_modelposix_defaulton&newcheck=postgres_REL_13_0_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_modelposix_defaulton&newcheck=postgres_REL_13_0_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | xerces | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_modelposix_defaulton&newcheck=xerces_v3.2.3_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_modelposix_defaulton&newcheck=xerces_v3.2.3_modelposix_defaultoff&is-unique=on&diff-mode=Resolved) | | bitcoin | [new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_modelposix_defaulton&newcheck=bitcoin_v0.20.1_modelposix_defaultoff&is-unique=on&diff-mode=New) | [lost reports](https://codechecker-demo.ea
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
NagyDonat wrote:
I analyzed the results uploaded by @balazske and found the following:
### memcached
The new ModelPosix=true produces two new bug reports [(1) assuming that
fileno() can
fail](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=memcached_1.6.8_modelposix_defaulton&newcheck=memcached_1.6.8_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464001&report-hash=2bf08110160cdf74b43d1443a243c170&report-filepath=%2aauthfile.c)
and [(2) errno is undefined after
close()](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=memcached_1.6.8_modelposix_defaulton&newcheck=memcached_1.6.8_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464024&report-hash=0317376f1ccbb4ad49347cc505972a1e&report-filepath=%2amemcached.c).
These are arguably true positives, although it's unclear whether fileno() can
fail or not ("These functions should not fail and do not set the external
variable errno. (However, in case fileno() detects that its argument is not
a valid stream, it must return -1 and set errno to EBADF.)" -- e.g. the manpage
on my linux claims both that it should not fail and that it can fail.).
### tmux
The new ModelPosix=true produces [yet another errno undefined after
close()](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=tmux_2.6_modelposix_defaulton&newcheck=tmux_2.6_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464058&report-hash=c2945236a92091b2bc3f3e45831171b9&report-filepath=%2aclient.c)
and a case where the [checker assumes that opening "/dev/null" can
fail](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=tmux_2.6_modelposix_defaulton&newcheck=tmux_2.6_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464065&report-hash=1ad575b0810ef8658405d7e625d05d55&report-filepath=%2acmd-pipe-pane.c).
The first is a TP, the second is FP in practice but is a reasonable report.
### curl
There are 9 new reports with ModelPosix=true:
- one very confusing report on an [extraordinarily ugly
macro](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464096&report-hash=7f1932ded5a624fc700c23f9773b2e8a&report-filepath=%2asockfilt.c)
-- probably FP, but the author "asked for it" with this mess,
- there are two
[bitwiseshift](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464097&report-hash=246fc7928949c326a376755a369179de&report-filepath=%2asockfilt.c)
[reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464098&report-hash=f6f35cd5508bdbb8bf7f63bd72193217&report-filepath=%2asockfilt.c)
on ugly black magic that breaks if we assume that `fileno()` returns `-1`,
- one that looks like a [straightforward TP caught among confusing code
branches](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464099&report-hash=710c0fca72ffec642cbcfcc8311a8fbd&report-filepath=%2asockfilt.c),
- a [71-step
monster](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464102&report-hash=d4a4bda38c5a6fdaabe2c1867158b106&report-filepath=%2atftpd.c)
that's also probably TP, but hard to understand,
- two straightforward failure of "`open()`" not checked reports
[(1)](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464104&report-hash=6a154ab1f902166b53ddfe5d0683d270&report-filepath=%2alib568.c)
[(2)](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464107&report-hash=84e8d41aa8e091f6d738fedccc48fe50&report-filepath=%2alib572.c)
in test code, these seem to be TPs
- an [`isatty(fileno())`
issue](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_modelposix_defaulton&newcheck=curl_curl-7_66_0_modelposix_defaultoff&is-unique=on&diff-type=Resolved&report-id=3464106&report-hash=711f723dd493d590bceee945f32c564b&report-filepath=%2atool_operate.c),
- a [`fstat(fileno(), ...)`
issue](https://codechecker-demo.eastus
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
balazske wrote: The new appeared bug reports should be similar to the ones that were observed when `StdCLibraryFunctionsChecker` was made non-alpha (and probably were checked already one time) (because the option was turned on in those tests). A different solution can be to add a Linux-mode for the checker (change option `ModelPOSIX` to an enumeration like "C", "POSIX", "Linux"). The strict POSIX standard does not tell exactly that `fileno` fails only if the file descriptor is invalid. Probably for other functions too the man pages are more detailed about error cases, so the information increases from C to POSIX to Linux. It may be possible to automatically detect presence of Linux source code by checking some macros. https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
balazske wrote: > * a [straightforward leak of a string returned by > `strdup()`](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_modelposix_defaulton&newcheck=postgres_REL_13_0_modelposix_defaultoff&is-unique=on&diff-type=New&report-id=3467892&report-hash=9278b17f14a2612356e847d5ef5426a0&report-filepath=%2aexec.c) > is lost and I don't know why. Perhaps turn this into an unit test to examine > what happens? This may happen because the "controlled environment" analyzer option may be set to `true` (but I did not check it). Without `ModelPOSIX` the `getenv` call can fail or not (it is not modeled), but with `ModelPOSIX` it is modeled by the checker and it is assumed that it can not fail (environment variable exists always). In this case the branch with `strdup` is not executed at all. https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
balazske wrote: Because the many cases with `fileno` I can agree to change the summary so we assume that it never fails. Probably an other checker may find a case if the passed file handle is invalid because it was not initialized, or the file was already closed (`StreamChecker` should find this). https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
NagyDonat wrote: > > * a [straightforward leak of a string returned by > > `strdup()`](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_modelposix_defaulton&newcheck=postgres_REL_13_0_modelposix_defaultoff&is-unique=on&diff-type=New&report-id=3467892&report-hash=9278b17f14a2612356e847d5ef5426a0&report-filepath=%2aexec.c) > > is lost and I don't know why. Perhaps turn this into an unit test to > > examine what happens? > > This may happen because the "controlled environment" analyzer option may be > set to `true` (but I did not check it). Without `ModelPOSIX` the `getenv` > call can fail or not (it is not modeled), but with `ModelPOSIX` it is modeled > by the checker and it is assumed that it can not fail (environment variable > exists always). In this case the branch with `strdup` is not executed at all. > Additionally this is maybe not a true positive. The string is passed to > `putenv` and probably should not be freed by the program. You're right that the string passed to `putenv` should not be freed, so this was a false positive. Let's just ignore the disappearance of this report, investigating it provides negligible benefits but could be difficult. > Because the many cases with `fileno` I can agree to change the summary so we > assume that it never fails. Thanks, that would be a good way forward. Ping me if you have a commit for changing the summary, I'll review it quickly. > Probably an other checker may find a case if the passed file handle is > invalid because it was not initialized, or the file was already closed > (`StreamChecker` should find this). Good idea, that would be very nice as a separate longer-term solution :) https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
https://github.com/balazske created
https://github.com/llvm/llvm-project/pull/80457
Default value of checker option `ModelPOSIX` is changed to `true`.
Documentation is updated.
From 1f65abda712efce624c01ec99675c8261a8e6cea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Wed, 31 Jan 2024 17:40:21 +0100
Subject: [PATCH] {clang][analyzer] Change default value of checker option in
unix.StdCLibraryFunctions.
Default value of checker option `ModelPOSIX` is changed to `true`.
Documentation is updated.
---
clang/docs/analyzer/checkers.rst | 19 +++
.../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +-
clang/test/Analysis/analyzer-config.c | 2 +-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index bb637cf1b8007..24522e56501e5 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1299,10 +1299,21 @@ 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.
+The ``ModelPOSIX`` option controls if functions from the POSIX standard are
+recognized by the checker. If ``true``, a big amount of POSIX functions is
+modeled according to the
+`POSIX standard`_. This
+includes ranges of parameters and possible return values. Furthermore the
+behavior related to ``errno`` in the POSIX case is often that ``errno`` is set
+only if a function call fails, and it becomes undefined after a successful
+function call.
+If ``false``, functions are modeled according to the C99 language standard.
+This includes far less functions than the POSIX case. It is possible that the
+same functions are modeled differently in the two cases because differences in
+the standards. The C standard specifies less aspects of the functions, for
+example exact ``errno`` behavior is often unspecified (and not modeled by the
+checker).
+Default value of the option is ``true``.
.. _osx-checkers:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index e7774e5a9392d..a224b81c33a62 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -578,7 +578,7 @@ def StdCLibraryFunctionsChecker :
Checker<"StdCLibraryFunctions">,
"ModelPOSIX",
"If set to true, the checker models additional functions "
"from the POSIX standard.",
- "false",
+ "true",
InAlpha>
]>,
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
diff --git a/clang/test/Analysis/analyzer-config.c
b/clang/test/Analysis/analyzer-config.c
index 373017f4b18bf..2167a2b32f596 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -129,7 +129,7 @@
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
// CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
-// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false
+// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = true
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: verbose-report-filename = false
// CHECK-NEXT: widen-loops = false
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
llvmbot wrote: @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) Changes Default value of checker option `ModelPOSIX` is changed to `true`. Documentation is updated. --- Full diff: https://github.com/llvm/llvm-project/pull/80457.diff 3 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+15-4) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+1-1) - (modified) clang/test/Analysis/analyzer-config.c (+1-1) ``diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index bb637cf1b8007..24522e56501e5 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1299,10 +1299,21 @@ 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. +The ``ModelPOSIX`` option controls if functions from the POSIX standard are +recognized by the checker. If ``true``, a big amount of POSIX functions is +modeled according to the +`POSIX standard`_. This +includes ranges of parameters and possible return values. Furthermore the +behavior related to ``errno`` in the POSIX case is often that ``errno`` is set +only if a function call fails, and it becomes undefined after a successful +function call. +If ``false``, functions are modeled according to the C99 language standard. +This includes far less functions than the POSIX case. It is possible that the +same functions are modeled differently in the two cases because differences in +the standards. The C standard specifies less aspects of the functions, for +example exact ``errno`` behavior is often unspecified (and not modeled by the +checker). +Default value of the option is ``true``. .. _osx-checkers: diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index e7774e5a9392d..a224b81c33a62 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -578,7 +578,7 @@ def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">, "ModelPOSIX", "If set to true, the checker models additional functions " "from the POSIX standard.", - "false", + "true", InAlpha> ]>, WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>, diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 373017f4b18bf..2167a2b32f596 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -129,7 +129,7 @@ // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false // CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true // CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false -// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = false +// CHECK-NEXT: unix.StdCLibraryFunctions:ModelPOSIX = true // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: verbose-report-filename = false // CHECK-NEXT: widen-loops = false `` https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
steakhal wrote: I'm excited to see this change. I've not reviewed this yet. https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
