[clang] [Wunsafe-buffer-usage] Fix false positives in handling enums (PR #117370)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 22 20:48:01 PST 2024
https://github.com/mxms0 updated https://github.com/llvm/llvm-project/pull/117370
>From 8fed333cf4221dbf1826351da80164db5d209c21 Mon Sep 17 00:00:00 2001
From: mxms <mxms at google.com>
Date: Fri, 22 Nov 2024 15:09:07 -0500
Subject: [PATCH 1/2] [Wunsafe-buffer-usage] Fix false positives in handling
enums
Do not warn if the index is an enum and we an determine statically that
it's within bounds.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 +++++++
.../SemaCXX/warn-unsafe-buffer-usage-array.cpp | 17 +++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..addb724e2e2c9a 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -463,6 +463,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
return true;
}
+ // Array index wasn't an integer literal, let's see if it was an enum or
+ // something similar
+ const auto IntConst = Node.getIdx()->getIntegerConstantExpr(Finder->getASTContext());
+ if (IntConst && *IntConst > 0 && *IntConst < size) {
+ return true;
+ }
+
return false;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..a65ecdf39edfcc 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -39,6 +39,23 @@ void constant_idx_unsafe(unsigned idx) {
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+enum FooEnum {
+ A = 0,
+ B = 1,
+ C = 2,
+ D
+};
+
+void constant_enum_safe() {
+ int buffer[FooEnum::D] = { 0, 1, 2 };
+ buffer[C] = 0; // no-warning
+}
+
+void constant_enum_unsafe(FooEnum e) {
+ int buffer[FooEnum::D] = { 0, 1, 2 };
+ buffer[e] = 0; // expected-warning{{unsafe buffer access}}
+}
+
void constant_id_string(unsigned idx) {
char safe_char = "abc"[1]; // no-warning
safe_char = ""[0];
>From 35207ea84425902a70b46f153e9619cc9d544f46 Mon Sep 17 00:00:00 2001
From: mxms <mxms at google.com>
Date: Fri, 22 Nov 2024 23:45:39 -0500
Subject: [PATCH 2/2] Detect when the buffer is a member access, fix tests
Add a case for when it's a member variable access and we can statically
determine the size. Also add new test to ensure the change works
reliably and update old tests to not expect this warning.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +-
.../warn-unsafe-buffer-usage-array.cpp | 5 ++++-
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 21 +++++++++----------
4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index addb724e2e2c9a..c005b34c2f63ad 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -466,7 +466,7 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// Array index wasn't an integer literal, let's see if it was an enum or
// something similar
const auto IntConst = Node.getIdx()->getIntegerConstantExpr(Finder->getASTContext());
- if (IntConst && *IntConst > 0 && *IntConst < size) {
+ if (IntConst && *IntConst >= 0 && *IntConst < size) {
return true;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index a65ecdf39edfcc..f5672db18de0ca 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -47,8 +47,11 @@ enum FooEnum {
};
void constant_enum_safe() {
- int buffer[FooEnum::D] = { 0, 1, 2 };
+ int buffer[FooEnum::D] = { 0, 1, 2 }; // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'buffer' to 'std::array' to label it for hardening}}
+ buffer[A] = 0; // no-warning
buffer[C] = 0; // no-warning
+ buffer[D] = 0; // expected-note{{used in buffer access here}}
}
void constant_enum_unsafe(FooEnum e) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}}
- //expected-warning at +1{{unsafe buffer access}}
v = d.buf[5]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}}
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index 642db0e9d3c632..41194a8e3f5222 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -128,25 +128,25 @@ T_t funRetT();
T_t * funRetTStar();
void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
- foo(sp->a[1], // expected-warning{{unsafe buffer access}}
+ foo(sp->a[1],
sp->b[1], // expected-warning{{unsafe buffer access}}
- sp->c.a[1], // expected-warning{{unsafe buffer access}}
+ sp->c.a[1],
sp->c.b[1], // expected-warning{{unsafe buffer access}}
- s.a[1], // expected-warning{{unsafe buffer access}}
+ s.a[1],
s.b[1], // expected-warning{{unsafe buffer access}}
- s.c.a[1], // expected-warning{{unsafe buffer access}}
+ s.c.a[1],
s.c.b[1], // expected-warning{{unsafe buffer access}}
- sp2->a[1], // expected-warning{{unsafe buffer access}}
+ sp2->a[1],
sp2->b[1], // expected-warning{{unsafe buffer access}}
- sp2->c.a[1], // expected-warning{{unsafe buffer access}}
+ sp2->c.a[1],
sp2->c.b[1], // expected-warning{{unsafe buffer access}}
- s2.a[1], // expected-warning{{unsafe buffer access}}
+ s2.a[1],
s2.b[1], // expected-warning{{unsafe buffer access}}
- s2.c.a[1], // expected-warning{{unsafe buffer access}}
+ s2.c.a[1],
s2.c.b[1], // expected-warning{{unsafe buffer access}}
- funRetT().a[1], // expected-warning{{unsafe buffer access}}
+ funRetT().a[1],
funRetT().b[1], // expected-warning{{unsafe buffer access}}
- funRetTStar()->a[1], // expected-warning{{unsafe buffer access}}
+ funRetTStar()->a[1],
funRetTStar()->b[1] // expected-warning{{unsafe buffer access}}
);
}
@@ -213,7 +213,6 @@ void testTypedefs(T_ptr_t p) {
// expected-warning at -1{{'p' is an unsafe pointer used for buffer access}}
foo(p[1], // expected-note{{used in buffer access here}}
p[1].a[1], // expected-note{{used in buffer access here}}
- // expected-warning at -1{{unsafe buffer access}}
p[1].b[1] // expected-note{{used in buffer access here}}
// expected-warning at -1{{unsafe buffer access}}
);
More information about the cfe-commits
mailing list