=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,DonatNagyE
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE closed
https://github.com/llvm/llvm-project/pull/72107
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,DonatNagyE
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From
https://github.com/Xazax-hun edited
https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Xazax-hun approved this pull request.
I wish we could clean up `checkLocation` to work as expected for all cases. It
is more future proof in the sense that if C++ introduces new kinds of
statements or expressions that could index into something (like
multi-dimensional
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -413,6 +464,19 @@ bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S,
ASTContext ) {
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/steakhal approved this pull request.
Sorry for reporting back only after a week or so. The analysis
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/steakhal edited
https://github.com/llvm/llvm-project/pull/72107
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
DonatNagyE wrote:
@Xazax-hun I added a few testcases that show the current behavior of the
checker w.r.t. invalid
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From ab102e949994a4462382e4c10c0153d61fb00306 Mon Sep 17
Xazax-hun wrote:
I think another question is whether warning on code like `int& val = arr[i]`
where `val` is not actually used on the path where `i == size` is OK. I would
not block this PR on a case like this, but it might be nice to add a test and
potentially a comment about the desired
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
DonatNagyE wrote:
Note that (if I understand it correctly) after this change the `ArrayBoundV2`
check may be triggered at a slightly earlier point of the analysis and
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
steakhal wrote:
> @steakhal thanks for the checking and sorry for the unintentional spamming.
>
> > Such a great feature, right?
>
> Just wonderful
To clarify, you
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy
Message-ID:
In-Reply-To:
DonatNagyE wrote:
@steakhal thanks for the checking and sorry for the unintentional spamming.
> Such a great feature, right?
Just wonderful :smile:
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -350,17 +383,38 @@ void ArrayBoundCheckerV2::checkLocation(SVal Location,
bool IsLoad,
if (ExceedsUpperBound) {
if (!WithinUpperBound) {
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
steakhal wrote:
FYI I edited the PR summary so that I'm not tagged there directly because if
someone is tagged in a commit message on GH, that person will be notified
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/steakhal edited
https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
steakhal wrote:
I'm in favor of this change. I'll pull the patch downstream and report back how
it performed.
Coming back to the `[size]` example, actually I believe
@@ -350,17 +383,38 @@ void ArrayBoundCheckerV2::checkLocation(SVal Location,
bool IsLoad,
if (ExceedsUpperBound) {
if (!WithinUpperBound) {
// We know that the index definitely exceeds the upper bound.
-std::string RegName = getRegionName(Reg);
-
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
DonatNagyE wrote:
I updated the PR to allow `[size]` for creating the after-the-end pointer
of an array. My implementation interprets this corner case narrowly to limit
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From ab102e949994a4462382e4c10c0153d61fb00306 Mon Sep 17 00:00:00 2001
From:
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From ab102e949994a4462382e4c10c0153d61fb00306 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE edited
https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/DonatNagyE updated
https://github.com/llvm/llvm-project/pull/72107
>From ab102e949994a4462382e4c10c0153d61fb00306 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Mon, 13 Nov 2023
@@ -34,20 +34,37 @@ using llvm::formatv;
namespace {
enum OOB_Kind { OOB_Precedes, OOB_Exceeds, OOB_Taint };
-class ArrayBoundCheckerV2 :
-public Checker {
+struct Messages {
+ std::string Short, Full;
+};
+
+class ArrayBoundCheckerV2 : public Checker,
+
@@ -64,6 +100,28 @@ double arrayInStructPtr(struct vec *pv) {
// expected-note@-2 {{Access of the field 'elems' at index 64, while it
holds only 64 'double' elements}}
}
+struct item {
+ int a, b;
+} itemArray[20] = {0};
+
+int structOfArrays(void) {
DonatNagyE wrote:
> Note that [idx] is perfectly valid code when `idx == number of
> elements`. And it is relatively common to do that when one is using STL
> algorithms on arrays:
>
> ```
> auto it = std::find([0], [size], foo);
> ```
>
> Of course, one could use the `begin/end` free
@@ -34,20 +34,37 @@ using llvm::formatv;
namespace {
enum OOB_Kind { OOB_Precedes, OOB_Exceeds, OOB_Taint };
-class ArrayBoundCheckerV2 :
-public Checker {
+struct Messages {
+ std::string Short, Full;
+};
+
+class ArrayBoundCheckerV2 : public Checker,
+
Xazax-hun wrote:
Note that [idx] is perfectly valid code when `idx == number of elements`.
And it is relatively common to do that when one is using STL algorithms on
arrays:
```
auto it = std::find([0], [size], foo);
```
Of course, one could use the `begin/end` free functions, but those are
@@ -34,20 +34,37 @@ using llvm::formatv;
namespace {
enum OOB_Kind { OOB_Precedes, OOB_Exceeds, OOB_Taint };
-class ArrayBoundCheckerV2 :
-public Checker {
+struct Messages {
+ std::string Short, Full;
+};
+
+class ArrayBoundCheckerV2 : public Checker,
+
@@ -64,6 +100,28 @@ double arrayInStructPtr(struct vec *pv) {
// expected-note@-2 {{Access of the field 'elems' at index 64, while it
holds only 64 'double' elements}}
}
+struct item {
+ int a, b;
+} itemArray[20] = {0};
+
+int structOfArrays(void) {
DonatNagyE wrote:
I evaluated a mostly [1] clean analysis run and it reveals that this change has
almost no effects when CSA analyses stable open source code (this is the
expected behavior, stable code doesn't contain out of bounds memory access).
The only differences are apparently caused by
DonatNagyE wrote:
I ran an analysis that compares this commit with its parent on many open source
projects. The results revealed that this commit "converts" many
alpha.security.ArrayBound (V1) results into alpha.security.ArrayBoundV2 results
because (if I understand it correctly) the new
https://github.com/DonatNagyE edited
https://github.com/llvm/llvm-project/pull/72107
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (DonatNagyE)
Changes
...instead of the currently used, more abstract Location callback. The main
advantage of this change is that after it the checker will check
`array[index].field` while the previous implementation ignored this
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: None (DonatNagyE)
Changes
...instead of the currently used, more abstract Location callback. The main
advantage of this change is that after it the checker will check
`array[index].field` while the previous
https://github.com/DonatNagyE created
https://github.com/llvm/llvm-project/pull/72107
...instead of the currently used, more abstract Location callback. The main
advantage of this change is that after it the checker will check
`array[index].field` while the previous implementation ignored
37 matches
Mail list logo