From: Benjamin Priour <vultk...@gcc.gnu.org>
This patch enchances -Wanalyzer-out-of-bounds that is no longer paired
with a -Wanalyzer-use-of-uninitialized-value on out-of-bounds-read.
This also fixes PR analyzer/109437.
Before there could always be at most one OOB-read warning per frame because
-Wanalyzer-use-of-uninitialized-value always terminates the analysis
path.
PR analyzer/109439
gcc/analyzer/ChangeLog:
* bounds-checking.cc (region_model::check_symbolic_bounds):
Returns whether the BASE_REG region access was OOB.
(region_model::check_region_bounds): Likewise.
* region-model.cc (region_model::get_store_value): Creates an
unknown svalue on OOB-read access to REG.
(region_model::check_region_access): Returns whether an
unknown svalue needs be created.
(region_model::check_region_for_read): Passes
check_region_access return value.
* region-model.h: Update prior function definitions.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/out-of-bounds-2.c: Cleaned test for
uninitialized-value warning.
* gcc.dg/analyzer/out-of-bounds-5.c: Likewise.
* gcc.dg/analyzer/pr101962.c: Likewise.
* gcc.dg/analyzer/realloc-5.c: Likewise.
* gcc.dg/analyzer/pr109439.c: New test.
---
gcc/analyzer/bounds-checking.cc | 30 +++++++++++++------
gcc/analyzer/region-model.cc | 22 +++++++++-----
gcc/analyzer/region-model.h | 8 ++---
.../gcc.dg/analyzer/out-of-bounds-2.c | 1 -
.../gcc.dg/analyzer/out-of-bounds-5.c | 2 --
gcc/testsuite/gcc.dg/analyzer/pr101962.c | 1 -
gcc/testsuite/gcc.dg/analyzer/pr109439.c | 12 ++++++++
gcc/testsuite/gcc.dg/analyzer/realloc-5.c | 1 -
8 files changed, 51 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr109439.c
diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index 3bf542a8eba..479b8e4b88d 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -767,9 +767,11 @@ public:
}
};
-/* Check whether an access is past the end of the BASE_REG. */
+/* Check whether an access is past the end of the BASE_REG.
+ Return TRUE if the access was valid, FALSE otherwise.
+*/
-void
+bool
region_model::check_symbolic_bounds (const region *base_reg,
const svalue *sym_byte_offset,
const svalue *num_bytes_sval,
@@ -800,6 +802,7 @@ region_model::check_symbolic_bounds (const region *base_reg,
offset_tree,
num_bytes_tree,
capacity_tree));
+ return false;
break;
case DIR_WRITE:
ctxt->warn (make_unique<symbolic_buffer_overflow> (base_reg,
@@ -807,9 +810,11 @@ region_model::check_symbolic_bounds (const region
*base_reg,
offset_tree,
num_bytes_tree,
capacity_tree));
+ return false;
break;
}
}
+ return true;
}
static tree
@@ -822,9 +827,11 @@ maybe_get_integer_cst_tree (const svalue *sval)
return NULL_TREE;
}
-/* May complain when the access on REG is out-of-bounds. */
+/* May complain when the access on REG is out-of-bounds.
+ Return TRUE if the access was valid, FALSE otherwise.
+*/
-void
+bool
region_model::check_region_bounds (const region *reg,
enum access_direction dir,
region_model_context *ctxt) const
@@ -839,14 +846,14 @@ region_model::check_region_bounds (const region *reg,
(e.g. because the analyzer did not see previous offsets on the latter,
it might think that a negative access is before the buffer). */
if (base_reg->symbolic_p ())
- return;
+ return true;
/* Find out how many bytes were accessed. */
const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr);
tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval);
/* Bail out if 0 bytes are accessed. */
if (num_bytes_tree && zerop (num_bytes_tree))
- return;
+ return true;
/* Get the capacity of the buffer. */
const svalue *capacity = get_capacity (base_reg);
@@ -877,13 +884,13 @@ region_model::check_region_bounds (const region *reg,
}
else
byte_offset_sval = reg_offset.get_symbolic_byte_offset ();
- check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval,
+ return check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval,
capacity, dir, ctxt);
- return;
}
/* Otherwise continue to check with concrete values. */
byte_range out (0, 0);
+ bool oob_safe = true;
/* NUM_BYTES_TREE should always be interpreted as unsigned. */
byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree);
byte_range read_bytes (offset, num_bytes_unsigned);
@@ -899,10 +906,12 @@ region_model::check_region_bounds (const region *reg,
case DIR_READ:
ctxt->warn (make_unique<concrete_buffer_under_read> (reg, diag_arg,
out));
+ oob_safe = false;
break;
case DIR_WRITE:
ctxt->warn (make_unique<concrete_buffer_underwrite> (reg, diag_arg,
out));
+ oob_safe = false;
break;
}
}
@@ -911,7 +920,7 @@ region_model::check_region_bounds (const region *reg,
do a symbolic check here because the inequality check does not reason
whether constants are greater than symbolic values. */
if (!cst_capacity_tree)
- return;
+ return oob_safe;
byte_range buffer (0, wi::to_offset (cst_capacity_tree));
/* If READ_BYTES exceeds BUFFER, we do have an overflow. */
@@ -929,13 +938,16 @@ region_model::check_region_bounds (const region *reg,
case DIR_READ:
ctxt->warn (make_unique<concrete_buffer_over_read> (reg, diag_arg,
out, byte_bound));
+ oob_safe = false;
break;
case DIR_WRITE:
ctxt->warn (make_unique<concrete_buffer_overflow> (reg, diag_arg,
out, byte_bound));
+ oob_safe = false;
break;
}
}
+ return oob_safe;
}
} // namespace ana
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 3bb3df2f063..fb96cd54940 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2396,7 +2396,8 @@ region_model::get_store_value (const region *reg,
if (reg->empty_p ())
return m_mgr->get_or_create_unknown_svalue (reg->get_type ());
- check_region_for_read (reg, ctxt);
+ if (check_region_for_read (reg, ctxt))
+ return m_mgr->get_or_create_unknown_svalue(reg->get_type());
/* Special-case: handle var_decls in the constant pool. */
if (const decl_region *decl_reg = reg->dyn_cast_decl_region ())
@@ -2802,19 +2803,22 @@ region_model::get_string_size (const region *reg) const
}
/* If CTXT is non-NULL, use it to warn about any problems accessing REG,
- using DIR to determine if this access is a read or write. */
+ using DIR to determine if this access is a read or write.
+ Return TRUE if an UNKNOWN_SVALUE needs be created. */
-void
+bool
region_model::check_region_access (const region *reg,
enum access_direction dir,
region_model_context *ctxt) const
{
/* Fail gracefully if CTXT is NULL. */
if (!ctxt)
- return;
+ return false;
+ bool need_unknown_sval = false;
check_region_for_taint (reg, dir, ctxt);
- check_region_bounds (reg, dir, ctxt);
+ if (!check_region_bounds (reg, dir, ctxt))
+ need_unknown_sval = true;
switch (dir)
{
@@ -2827,6 +2831,7 @@ region_model::check_region_access (const region *reg,
check_for_writable_region (reg, ctxt);
break;
}
+ return need_unknown_sval;
}
/* If CTXT is non-NULL, use it to warn about any problems writing to REG. */
@@ -2838,13 +2843,14 @@ region_model::check_region_for_write (const region
*dest_reg,
check_region_access (dest_reg, DIR_WRITE, ctxt);
}
-/* If CTXT is non-NULL, use it to warn about any problems reading from REG. */
+/* If CTXT is non-NULL, use it to warn about any problems reading from REG.
+ Returns TRUE if an unknown svalue needs be created. */
-void
+bool
region_model::check_region_for_read (const region *src_reg,
region_model_context *ctxt) const
{
- check_region_access (src_reg, DIR_READ, ctxt);
+ return check_region_access (src_reg, DIR_READ, ctxt);
}
/* Concrete subclass for casts of pointers that lead to trailing bytes. */
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index fe3db0b0c98..12f84b20463 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -553,22 +553,22 @@ private:
void check_for_writable_region (const region* dest_reg,
region_model_context *ctxt) const;
- void check_region_access (const region *reg,
+ bool check_region_access (const region *reg,
enum access_direction dir,
region_model_context *ctxt) const;
- void check_region_for_read (const region *src_reg,
+ bool check_region_for_read (const region *src_reg,
region_model_context *ctxt) const;
void check_region_size (const region *lhs_reg, const svalue *rhs_sval,
region_model_context *ctxt) const;
/* Implemented in bounds-checking.cc */
- void check_symbolic_bounds (const region *base_reg,
+ bool check_symbolic_bounds (const region *base_reg,
const svalue *sym_byte_offset,
const svalue *num_bytes_sval,
const svalue *capacity,
enum access_direction dir,
region_model_context *ctxt) const;
- void check_region_bounds (const region *reg, enum access_direction dir,
+ bool check_region_bounds (const region *reg, enum access_direction dir,
region_model_context *ctxt) const;
void check_call_args (const call_details &cd) const;
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
index 1330090f348..336f624441c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
@@ -82,5 +82,4 @@ void test5 (void)
/* { dg-warning "heap-based buffer over-read" "bounds warning" { target
*-*-* } test5 } */
/* { dg-message "read of 4 bytes from after the end of the region" "num bad
bytes note" { target *-*-* } test5 } */
- /* { dg-warning "use of uninitialized value" "uninit warning" { target *-*-*
} test5 } */
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
index 2a61d8ca236..568f9cad199 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
@@ -68,7 +68,6 @@ void test8 (size_t size, size_t offset)
char dst[size];
memcpy (dst, src, size + offset); /* { dg-line test8 } */
/* { dg-warning "over-read" "warning" { target *-*-* } test8 } */
- /* { dg-warning "use of uninitialized value" "warning" { target *-*-* }
test8 } */
/* { dg-warning "overflow" "warning" { target *-*-* } test8 } */
}
@@ -78,7 +77,6 @@ void test9 (size_t size, size_t offset)
int32_t dst[size];
memcpy (dst, src, 4 * size + 1); /* { dg-line test9 } */
/* { dg-warning "over-read" "warning" { target *-*-* } test9 } */
- /* { dg-warning "use of uninitialized value" "warning" { target *-*-* }
test9 } */
/* { dg-warning "overflow" "warning" { target *-*-* } test9 } */
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c
b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
index 08c0aba5cbf..b878aad9cf1 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
@@ -24,7 +24,6 @@ test_1 (void)
__analyzer_eval (a != NULL); /* { dg-warning "TRUE" } */
return *a; /* { dg-line test_1 } */
- /* { dg-warning "use of uninitialized value '\\*a'" "warning" { target *-*-*
} test_1 } */
/* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* }
test_1 } */
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr109439.c
b/gcc/testsuite/gcc.dg/analyzer/pr109439.c
new file mode 100644
index 00000000000..01c87cf171c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr109439.c
@@ -0,0 +1,12 @@
+int would_like_only_oob (int i)
+{
+ int arr[] = {1,2,3,4,5,6,7};
+ arr[10] = 9; /* { dg-warning "stack-based buffer overflow" } */
+ arr[11] = 15; /* { dg-warning "stack-based buffer overflow" } */
+ int y1 = arr[9]; /* { dg-warning "stack-based buffer over-read" } */
+ /* { dg-bogus "use of uninitialized value" "" { target *-*-* } .-1
} */
+
+ arr[18] = 15; /* { dg-warning "stack-based buffer overflow" } */
+
+ return y1;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
index 137e05b87aa..f65f2c6ca25 100644
--- a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
+++ b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
@@ -40,7 +40,6 @@ void test_1 ()
/* { dg-warning "UNKNOWN" "warning" { target *-*-* } eval } */
/* { dg-warning "heap-based buffer over-read" "warning" { target *-*-* }
eval } */
- /* { dg-warning "use of uninitialized value" "warning" { target *-*-* }
eval } */
}
free (q);
--
2.34.1
