PR analyzer/112792 reports false positives from -fanalyzer's
bounds-checking on certain packed structs containing bitfields e.g.
in the Linux kernel's drivers/dma/idxd/device.c:
union msix_perm {
struct {
u32 rsvd2 : 8;
u32 pasid : 20;
};
u32 bits;
} __attribute__((__packed__));
The root cause is that the bounds-checking is done using byte offsets
and ranges; in the above, an access of "pasid" is treated as a 32-bit
access starting one byte inside the union, thus accessing byte offsets
1-4 when only offsets 0-3 are valid.
This patch updates the bounds-checking to use bit offsets and ranges
wherever possible - for concrete offsets and capacities. In the above
accessing "pasid" is treated as bits 8-27 of a 32-bit region, fixing the
false positive.
Symbolic offsets and ranges are still handled at byte granularity.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Successful run of analyzer integration tests on x86_64-pc-linux-gnu.
Pushed to trunk as r14-6622-g5f1bed2a7af828.
gcc/analyzer/ChangeLog:
PR analyzer/112792
* bounds-checking.cc
(out_of_bounds::oob_region_creation_event_capacity): Rename
"capacity" to "byte_capacity". Layout fix.
(out_of_bounds::::add_region_creation_events): Rename
"capacity" to "byte_capacity".
(class concrete_out_of_bounds): Rename m_out_of_bounds_range to
m_out_of_bounds_bits and convert from a byte_range to a bit_range.
(concrete_out_of_bounds::get_out_of_bounds_bytes): New.
(concrete_past_the_end::concrete_past_the_end): Rename param
"byte_bound" to "bit_bound". Initialize m_byte_bound.
(concrete_past_the_end::subclass_equal_p): Update for renaming
of m_byte_bound to m_bit_bound.
(concrete_past_the_end::m_bit_bound): New field.
(concrete_buffer_overflow::concrete_buffer_overflow): Convert
param "range" from byte_range to bit_range. Rename param
"byte_bound" to "bit_bound".
(concrete_buffer_overflow::emit): Update for bits vs bytes.
(concrete_buffer_overflow::describe_final_event): Split
into...
(concrete_buffer_overflow::describe_final_event_as_bytes): ...this
(concrete_buffer_overflow::describe_final_event_as_bits): ...and
this.
(concrete_buffer_over_read::concrete_buffer_over_read): Convert
param "range" from byte_range to bit_range. Rename param
"byte_bound" to "bit_bound".
(concrete_buffer_over_read::emit): Update for bits vs bytes.
(concrete_buffer_over_read::describe_final_event): Split into...
(concrete_buffer_over_read::describe_final_event_as_bytes):
...this
(concrete_buffer_over_read::describe_final_event_as_bits): ...and
this.
(concrete_buffer_underwrite::concrete_buffer_underwrite): Convert
param "range" from byte_range to bit_range.
(concrete_buffer_underwrite::describe_final_event): Split into...
(concrete_buffer_underwrite::describe_final_event_as_bytes):
...this
(concrete_buffer_underwrite::describe_final_event_as_bits): ...and
this.
(concrete_buffer_under_read::concrete_buffer_under_read): Convert
param "range" from byte_range to bit_range.
(concrete_buffer_under_read::describe_final_event): Split into...
(concrete_buffer_under_read::describe_final_event_as_bytes):
...this
(concrete_buffer_under_read::describe_final_event_as_bits): ...and
this.
(region_model::check_region_bounds): Use bits for concrete values,
and rename locals to indicate whether we're dealing with bits or
bytes. Specifically, replace "num_bytes_sval" with
"num_bits_sval", and get it from reg's "get_bit_size_sval".
Replace "num_bytes_tree" with "num_bits_tree". Rename "capacity"
to "byte_capacity". Rename "cst_capacity_tree" to
"cst_byte_capacity_tree". Replace "offset" and
"num_bytes_unsigned" with "bit_offset" and "num_bits_unsigned"
respectively, converting from byte_offset_t to bit_offset_t.
Replace "out" and "read_bytes" with "bits_outside" and "read_bits"
respectively, converting from byte_range to bit_range. Convert
"buffer" from byte_range to bit_range. Replace "byte_bound" with
"bit_bound".
* region.cc (region::get_bit_size_sval): New.
(offset_region::get_bit_offset): New.
(offset_region::get_bit_size_sval): New.
(sized_region::get_bit_size_sval): New.
(bit_range_region::get_bit_size_sval): New.
* region.h (region::get_bit_size_sval): New vfunc.
(offset_region::get_bit_offset): New decl.
(offset_region::get_bit_size_sval): New decl.
(sized_region::get_bit_size_sval): New decl.
(bit_range_region::get_bit_size_sval): New decl.
* store.cc (bit_range::intersects_p): New, based on
byte_range::intersects_p.
(bit_range::exceeds_p): New, based on byte_range::exceeds_p.
(bit_range::falls_short_of_p): New, based on
byte_range::falls_short_of_p.
(byte_range::intersects_p): Delete.
(byte_range::exceeds_p): Delete.
(byte_range::falls_short_of_p): Delete.
* store.h (bit_range::intersects_p): New overload.
(bit_range::exceeds_p): New.
(bit_range::falls_short_of_p): New.
(byte_range::intersects_p): Delete.
(byte_range::exceeds_p): Delete.
(byte_range::falls_short_of_p): Delete.
gcc/testsuite/ChangeLog:
PR analyzer/112792
* c-c++-common/analyzer/out-of-bounds-pr112792.c: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
---
gcc/analyzer/bounds-checking.cc | 435 ++++++++++++++----
gcc/analyzer/region.cc | 71 +++
gcc/analyzer/region.h | 12 +-
gcc/analyzer/store.cc | 142 +++---
gcc/analyzer/store.h | 17 +-
.../analyzer/out-of-bounds-pr112792.c | 18 +
6 files changed, 512 insertions(+), 183 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr112792.c
diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index cc43ecc54683..7cbfe91515f8 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -50,12 +50,12 @@ public:
class oob_region_creation_event_capacity : public
region_creation_event_capacity
{
public:
- oob_region_creation_event_capacity (tree capacity,
+ oob_region_creation_event_capacity (tree byte_capacity,
const event_loc_info &loc_info,
out_of_bounds &oob)
- : region_creation_event_capacity (capacity,
- loc_info),
- m_oob (oob)
+ : region_creation_event_capacity (byte_capacity,
+ loc_info),
+ m_oob (oob)
{
}
void prepare_for_emission (checker_path *path,
@@ -67,7 +67,7 @@ public:
emission_id);
m_oob.m_region_creation_event_id = emission_id;
}
- private:
+ private:
out_of_bounds &m_oob;
};
@@ -97,15 +97,16 @@ public:
}
void add_region_creation_events (const region *,
- tree capacity,
+ tree byte_capacity,
const event_loc_info &loc_info,
checker_path &emission_path) override
{
/* The memory space is described in the diagnostic message itself,
so we don't need an event for that. */
- if (capacity)
+ if (byte_capacity)
emission_path.add_event
- (make_unique<oob_region_creation_event_capacity> (capacity, loc_info,
+ (make_unique<oob_region_creation_event_capacity> (byte_capacity,
+ loc_info,
*this));
}
@@ -205,10 +206,10 @@ class concrete_out_of_bounds : public out_of_bounds
public:
concrete_out_of_bounds (const region_model &model,
const region *reg, tree diag_arg,
- byte_range out_of_bounds_range,
+ bit_range out_of_bounds_bits,
const svalue *sval_hint)
: out_of_bounds (model, reg, diag_arg, sval_hint),
- m_out_of_bounds_range (out_of_bounds_range)
+ m_out_of_bounds_bits (out_of_bounds_bits)
{}
bool subclass_equal_p (const pending_diagnostic &base_other) const override
@@ -216,11 +217,16 @@ public:
const concrete_out_of_bounds &other
(static_cast <const concrete_out_of_bounds &>(base_other));
return (out_of_bounds::subclass_equal_p (other)
- && m_out_of_bounds_range == other.m_out_of_bounds_range);
+ && m_out_of_bounds_bits == other.m_out_of_bounds_bits);
+ }
+
+ bool get_out_of_bounds_bytes (byte_range *out) const
+ {
+ return m_out_of_bounds_bits.as_byte_range (out);
}
protected:
- byte_range m_out_of_bounds_range;
+ bit_range m_out_of_bounds_bits;
};
/* Abstract subclass to complaing about concrete out-of-bounds
@@ -230,12 +236,18 @@ class concrete_past_the_end : public
concrete_out_of_bounds
{
public:
concrete_past_the_end (const region_model &model,
- const region *reg, tree diag_arg, byte_range range,
- tree byte_bound,
+ const region *reg, tree diag_arg, bit_range range,
+ tree bit_bound,
const svalue *sval_hint)
: concrete_out_of_bounds (model, reg, diag_arg, range, sval_hint),
- m_byte_bound (byte_bound)
- {}
+ m_bit_bound (bit_bound),
+ m_byte_bound (NULL_TREE)
+ {
+ if (m_bit_bound && TREE_CODE (m_bit_bound) == INTEGER_CST)
+ m_byte_bound
+ = wide_int_to_tree (size_type_node,
+ wi::to_offset (m_bit_bound) >> LOG2_BITS_PER_UNIT);
+ }
bool
subclass_equal_p (const pending_diagnostic &base_other) const final override
@@ -243,8 +255,8 @@ public:
const concrete_past_the_end &other
(static_cast <const concrete_past_the_end &>(base_other));
return (concrete_out_of_bounds::subclass_equal_p (other)
- && pending_diagnostic::same_tree_p (m_byte_bound,
- other.m_byte_bound));
+ && pending_diagnostic::same_tree_p (m_bit_bound,
+ other.m_bit_bound));
}
void add_region_creation_events (const region *,
@@ -260,6 +272,7 @@ public:
}
protected:
+ tree m_bit_bound;
tree m_byte_bound;
};
@@ -270,9 +283,9 @@ class concrete_buffer_overflow : public
concrete_past_the_end
public:
concrete_buffer_overflow (const region_model &model,
const region *reg, tree diag_arg,
- byte_range range, tree byte_bound,
+ bit_range range, tree bit_bound,
const svalue *sval_hint)
- : concrete_past_the_end (model, reg, diag_arg, range, byte_bound, sval_hint)
+ : concrete_past_the_end (model, reg, diag_arg, range, bit_bound, sval_hint)
{}
const char *get_kind () const final override
@@ -301,23 +314,44 @@ public:
if (warned)
{
- if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes))
+ if (wi::fits_uhwi_p (m_out_of_bounds_bits.m_size_in_bits))
{
- unsigned HOST_WIDE_INT num_bad_bytes
- = m_out_of_bounds_range.m_size_in_bytes.to_uhwi ();
- if (m_diag_arg)
- inform_n (ctxt.get_location (),
- num_bad_bytes,
- "write of %wu byte to beyond the end of %qE",
- "write of %wu bytes to beyond the end of %qE",
- num_bad_bytes,
- m_diag_arg);
+ unsigned HOST_WIDE_INT num_bad_bits
+ = m_out_of_bounds_bits.m_size_in_bits.to_uhwi ();
+ if (num_bad_bits % BITS_PER_UNIT == 0)
+ {
+ unsigned HOST_WIDE_INT num_bad_bytes
+ = num_bad_bits / BITS_PER_UNIT;
+ if (m_diag_arg)
+ inform_n (ctxt.get_location (),
+ num_bad_bytes,
+ "write of %wu byte to beyond the end of %qE",
+ "write of %wu bytes to beyond the end of %qE",
+ num_bad_bytes,
+ m_diag_arg);
+ else
+ inform_n (ctxt.get_location (),
+ num_bad_bytes,
+ "write of %wu byte to beyond the end of the region",
+ "write of %wu bytes to beyond the end of the
region",
+ num_bad_bytes);
+ }
else
- inform_n (ctxt.get_location (),
- num_bad_bytes,
- "write of %wu byte to beyond the end of the region",
- "write of %wu bytes to beyond the end of the region",
- num_bad_bytes);
+ {
+ if (m_diag_arg)
+ inform_n (ctxt.get_location (),
+ num_bad_bits,
+ "write of %wu bit to beyond the end of %qE",
+ "write of %wu bits to beyond the end of %qE",
+ num_bad_bits,
+ m_diag_arg);
+ else
+ inform_n (ctxt.get_location (),
+ num_bad_bits,
+ "write of %wu bit to beyond the end of the region",
+ "write of %wu bits to beyond the end of the region",
+ num_bad_bits);
+ }
}
else if (m_diag_arg)
inform (ctxt.get_location (),
@@ -331,10 +365,23 @@ public:
}
label_text describe_final_event (const evdesc::final_event &ev)
- final override
+ final override
{
- byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
- byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+ if (m_byte_bound || !m_bit_bound)
+ {
+ byte_range out_of_bounds_bytes (0, 0);
+ if (get_out_of_bounds_bytes (&out_of_bounds_bytes))
+ return describe_final_event_as_bytes (ev, out_of_bounds_bytes);
+ }
+ return describe_final_event_as_bits (ev);
+ }
+
+ label_text
+ describe_final_event_as_bytes (const evdesc::final_event &ev,
+ const byte_range &out_of_bounds_bytes)
+ {
+ byte_size_t start = out_of_bounds_bytes.get_start_byte_offset ();
+ byte_size_t end = out_of_bounds_bytes.get_last_byte_offset ();
char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
print_dec (start, start_buf, SIGNED);
char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
@@ -345,10 +392,10 @@ public:
if (m_diag_arg)
return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
" ends at byte %E", start_buf, m_diag_arg,
- m_byte_bound);
+ m_byte_bound);
return ev.formatted_print ("out-of-bounds write at byte %s but region"
" ends at byte %E", start_buf,
- m_byte_bound);
+ m_byte_bound);
}
else
{
@@ -363,6 +410,38 @@ public:
}
}
+ label_text describe_final_event_as_bits (const evdesc::final_event &ev)
+ {
+ bit_size_t start = m_out_of_bounds_bits.get_start_bit_offset ();
+ bit_size_t end = m_out_of_bounds_bits.get_last_bit_offset ();
+ char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (start, start_buf, SIGNED);
+ char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (end, end_buf, SIGNED);
+
+ if (start == end)
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds write at bit %s but %qE"
+ " ends at bit %E", start_buf, m_diag_arg,
+ m_bit_bound);
+ return ev.formatted_print ("out-of-bounds write at bit %s but region"
+ " ends at bit %E", start_buf,
+ m_bit_bound);
+ }
+ else
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds write from bit %s till"
+ " bit %s but %qE ends at bit %E",
+ start_buf, end_buf, m_diag_arg,
+ m_bit_bound);
+ return ev.formatted_print ("out-of-bounds write from bit %s till"
+ " bit %s but region ends at bit %E",
+ start_buf, end_buf, m_bit_bound);
+ }
+ }
+
enum access_direction get_dir () const final override { return DIR_WRITE; }
};
@@ -373,8 +452,8 @@ class concrete_buffer_over_read : public
concrete_past_the_end
public:
concrete_buffer_over_read (const region_model &model,
const region *reg, tree diag_arg,
- byte_range range, tree byte_bound)
- : concrete_past_the_end (model, reg, diag_arg, range, byte_bound, NULL)
+ bit_range range, tree bit_bound)
+ : concrete_past_the_end (model, reg, diag_arg, range, bit_bound, NULL)
{}
const char *get_kind () const final override
@@ -401,23 +480,44 @@ public:
if (warned)
{
- if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes))
+ if (wi::fits_uhwi_p (m_out_of_bounds_bits.m_size_in_bits))
{
- unsigned HOST_WIDE_INT num_bad_bytes
- = m_out_of_bounds_range.m_size_in_bytes.to_uhwi ();
- if (m_diag_arg)
- inform_n (ctxt.get_location (),
- num_bad_bytes,
- "read of %wu byte from after the end of %qE",
- "read of %wu bytes from after the end of %qE",
- num_bad_bytes,
- m_diag_arg);
+ unsigned HOST_WIDE_INT num_bad_bits
+ = m_out_of_bounds_bits.m_size_in_bits.to_uhwi ();
+ if (num_bad_bits % BITS_PER_UNIT == 0)
+ {
+ unsigned HOST_WIDE_INT num_bad_bytes
+ = num_bad_bits / BITS_PER_UNIT;
+ if (m_diag_arg)
+ inform_n (ctxt.get_location (),
+ num_bad_bytes,
+ "read of %wu byte from after the end of %qE",
+ "read of %wu bytes from after the end of %qE",
+ num_bad_bytes,
+ m_diag_arg);
+ else
+ inform_n (ctxt.get_location (),
+ num_bad_bytes,
+ "read of %wu byte from after the end of the region",
+ "read of %wu bytes from after the end of the
region",
+ num_bad_bytes);
+ }
else
- inform_n (ctxt.get_location (),
- num_bad_bytes,
- "read of %wu byte from after the end of the region",
- "read of %wu bytes from after the end of the region",
- num_bad_bytes);
+ {
+ if (m_diag_arg)
+ inform_n (ctxt.get_location (),
+ num_bad_bits,
+ "read of %wu bit from after the end of %qE",
+ "read of %wu bits from after the end of %qE",
+ num_bad_bits,
+ m_diag_arg);
+ else
+ inform_n (ctxt.get_location (),
+ num_bad_bits,
+ "read of %wu bit from after the end of the region",
+ "read of %wu bits from after the end of the region",
+ num_bad_bits);
+ }
}
else if (m_diag_arg)
inform (ctxt.get_location (),
@@ -431,10 +531,23 @@ public:
}
label_text describe_final_event (const evdesc::final_event &ev)
- final override
+ final override
{
- byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
- byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+ if (m_byte_bound || !m_bit_bound)
+ {
+ byte_range out_of_bounds_bytes (0, 0);
+ if (get_out_of_bounds_bytes (&out_of_bounds_bytes))
+ return describe_final_event_as_bytes (ev, out_of_bounds_bytes);
+ }
+ return describe_final_event_as_bits (ev);
+ }
+
+ label_text
+ describe_final_event_as_bytes (const evdesc::final_event &ev,
+ const byte_range &out_of_bounds_bytes)
+ {
+ byte_size_t start = out_of_bounds_bytes.get_start_byte_offset ();
+ byte_size_t end = out_of_bounds_bytes.get_last_byte_offset ();
char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
print_dec (start, start_buf, SIGNED);
char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
@@ -463,6 +576,38 @@ public:
}
}
+ label_text describe_final_event_as_bits (const evdesc::final_event &ev)
+ {
+ bit_size_t start = m_out_of_bounds_bits.get_start_bit_offset ();
+ bit_size_t end = m_out_of_bounds_bits.get_last_bit_offset ();
+ char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (start, start_buf, SIGNED);
+ char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (end, end_buf, SIGNED);
+
+ if (start == end)
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds read at bit %s but %qE"
+ " ends at bit %E", start_buf, m_diag_arg,
+ m_bit_bound);
+ return ev.formatted_print ("out-of-bounds read at bit %s but region"
+ " ends at bit %E", start_buf,
+ m_bit_bound);
+ }
+ else
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds read from bit %s till"
+ " bit %s but %qE ends at bit %E",
+ start_buf, end_buf, m_diag_arg,
+ m_bit_bound);
+ return ev.formatted_print ("out-of-bounds read from bit %s till"
+ " bit %s but region ends at bit %E",
+ start_buf, end_buf, m_bit_bound);
+ }
+ }
+
enum access_direction get_dir () const final override { return DIR_READ; }
};
@@ -473,7 +618,7 @@ class concrete_buffer_underwrite : public
concrete_out_of_bounds
public:
concrete_buffer_underwrite (const region_model &model,
const region *reg, tree diag_arg,
- byte_range range,
+ bit_range range,
const svalue *sval_hint)
: concrete_out_of_bounds (model, reg, diag_arg, range, sval_hint)
{}
@@ -505,10 +650,20 @@ public:
}
label_text describe_final_event (const evdesc::final_event &ev)
- final override
+ final override
{
- byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
- byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+ byte_range out_of_bounds_bytes (0, 0);
+ if (get_out_of_bounds_bytes (&out_of_bounds_bytes))
+ return describe_final_event_as_bytes (ev, out_of_bounds_bytes);
+ return describe_final_event_as_bits (ev);
+ }
+
+ label_text
+ describe_final_event_as_bytes (const evdesc::final_event &ev,
+ const byte_range &out_of_bounds_bytes)
+ {
+ byte_size_t start = out_of_bounds_bytes.get_start_byte_offset ();
+ byte_size_t end = out_of_bounds_bytes.get_last_byte_offset ();
char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
print_dec (start, start_buf, SIGNED);
char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
@@ -518,8 +673,8 @@ public:
{
if (m_diag_arg)
return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
- " starts at byte 0", start_buf,
- m_diag_arg);
+ " starts at byte 0",
+ start_buf, m_diag_arg);
return ev.formatted_print ("out-of-bounds write at byte %s but region"
" starts at byte 0", start_buf);
}
@@ -535,6 +690,37 @@ public:
}
}
+ label_text
+ describe_final_event_as_bits (const evdesc::final_event &ev)
+ {
+ bit_size_t start = m_out_of_bounds_bits.get_start_bit_offset ();
+ bit_size_t end = m_out_of_bounds_bits.get_last_bit_offset ();
+ char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (start, start_buf, SIGNED);
+ char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (end, end_buf, SIGNED);
+
+ if (start == end)
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds write at bit %s but %qE"
+ " starts at bit 0",
+ start_buf, m_diag_arg);
+ return ev.formatted_print ("out-of-bounds write at bit %s but region"
+ " starts at bit 0", start_buf);
+ }
+ else
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds write from bit %s till"
+ " bit %s but %qE starts at bit 0",
+ start_buf, end_buf, m_diag_arg);
+ return ev.formatted_print ("out-of-bounds write from bit %s till"
+ " bit %s but region starts at bit 0",
+ start_buf, end_buf);;
+ }
+ }
+
enum access_direction get_dir () const final override { return DIR_WRITE; }
};
@@ -545,7 +731,7 @@ class concrete_buffer_under_read : public
concrete_out_of_bounds
public:
concrete_buffer_under_read (const region_model &model,
const region *reg, tree diag_arg,
- byte_range range)
+ bit_range range)
: concrete_out_of_bounds (model, reg, diag_arg, range, NULL)
{}
@@ -576,10 +762,20 @@ public:
}
label_text describe_final_event (const evdesc::final_event &ev)
- final override
+ final override
{
- byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
- byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+ byte_range out_of_bounds_bytes (0, 0);
+ if (get_out_of_bounds_bytes (&out_of_bounds_bytes))
+ return describe_final_event_as_bytes (ev, out_of_bounds_bytes);
+ return describe_final_event_as_bits (ev);
+ }
+
+ label_text
+ describe_final_event_as_bytes (const evdesc::final_event &ev,
+ const byte_range &out_of_bounds_bytes)
+ {
+ byte_size_t start = out_of_bounds_bytes.get_start_byte_offset ();
+ byte_size_t end = out_of_bounds_bytes.get_last_byte_offset ();
char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
print_dec (start, start_buf, SIGNED);
char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
@@ -606,6 +802,36 @@ public:
}
}
+ label_text describe_final_event_as_bits (const evdesc::final_event &ev)
+ {
+ bit_size_t start = m_out_of_bounds_bits.get_start_bit_offset ();
+ bit_size_t end = m_out_of_bounds_bits.get_last_bit_offset ();
+ char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (start, start_buf, SIGNED);
+ char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+ print_dec (end, end_buf, SIGNED);
+
+ if (start == end)
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds read at bit %s but %qE"
+ " starts at bit 0", start_buf,
+ m_diag_arg);
+ return ev.formatted_print ("out-of-bounds read at bit %s but region"
+ " starts at bit 0", start_buf);
+ }
+ else
+ {
+ if (m_diag_arg)
+ return ev.formatted_print ("out-of-bounds read from bit %s till"
+ " bit %s but %qE starts at bit 0",
+ start_buf, end_buf, m_diag_arg);
+ return ev.formatted_print ("out-of-bounds read from bit %s till"
+ " bit %s but region starts at bit 0",
+ start_buf, end_buf);;
+ }
+ }
+
enum access_direction get_dir () const final override { return DIR_READ; }
};
@@ -955,16 +1181,16 @@ region_model::check_region_bounds (const region *reg,
region_offset reg_offset = reg->get_offset (m_mgr);
const region *base_reg = reg_offset.get_base_region ();
- /* 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))
+ /* Find out how many bits were accessed. */
+ const svalue *num_bits_sval = reg->get_bit_size_sval (m_mgr);
+ tree num_bits_tree = maybe_get_integer_cst_tree (num_bits_sval);
+ /* Bail out if 0 bits are accessed. */
+ if (num_bits_tree && zerop (num_bits_tree))
return true;
- /* Get the capacity of the buffer. */
- const svalue *capacity = get_capacity (base_reg);
- tree cst_capacity_tree = maybe_get_integer_cst_tree (capacity);
+ /* Get the capacity of the buffer (in bytes). */
+ const svalue *byte_capacity = get_capacity (base_reg);
+ tree cst_byte_capacity_tree = maybe_get_integer_cst_tree (byte_capacity);
/* The constant offset from a pointer is represented internally as a sizetype
but should be interpreted as a signed value here. The statement below
@@ -973,36 +1199,39 @@ region_model::check_region_bounds (const region *reg,
For example, this is needed for out-of-bounds-3.c test1 to pass when
compiled with a 64-bit gcc build targeting 32-bit systems. */
- byte_offset_t offset;
+ bit_offset_t bit_offset;
if (!reg_offset.symbolic_p ())
- offset = wi::sext (reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT,
- TYPE_PRECISION (size_type_node));
+ bit_offset = wi::sext (reg_offset.get_bit_offset (),
+ TYPE_PRECISION (size_type_node));
/* If any of the base region, the offset, or the number of bytes accessed
are symbolic, we have to reason about symbolic values. */
- if (base_reg->symbolic_p () || reg_offset.symbolic_p () || !num_bytes_tree)
+ if (base_reg->symbolic_p () || reg_offset.symbolic_p () || !num_bits_tree)
{
const svalue* byte_offset_sval;
if (!reg_offset.symbolic_p ())
{
- tree offset_tree = wide_int_to_tree (integer_type_node, offset);
+ tree byte_offset_tree
+ = wide_int_to_tree (integer_type_node,
+ bit_offset >> LOG2_BITS_PER_UNIT);
byte_offset_sval
- = m_mgr->get_or_create_constant_svalue (offset_tree);
+ = m_mgr->get_or_create_constant_svalue (byte_offset_tree);
}
else
byte_offset_sval = reg_offset.get_symbolic_byte_offset ();
+ const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr);
return check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval,
- capacity, dir, sval_hint, ctxt);
+ byte_capacity, dir, sval_hint, ctxt);
}
/* Otherwise continue to check with concrete values. */
- byte_range out (0, 0);
+ bit_range bits_outside (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);
- /* If read_bytes has a subset < 0, we do have an underwrite. */
- if (read_bytes.falls_short_of_p (0, &out))
+ /* NUM_BITS_TREE should always be interpreted as unsigned. */
+ bit_offset_t num_bits_unsigned = wi::to_offset (num_bits_tree);
+ bit_range read_bits (bit_offset, num_bits_unsigned);
+ /* If read_bits has a subset < 0, we do have an underwrite. */
+ if (read_bits.falls_short_of_p (0, &bits_outside))
{
tree diag_arg = get_representative_tree (base_reg);
switch (dir)
@@ -1014,13 +1243,13 @@ region_model::check_region_bounds (const region *reg,
gcc_assert (sval_hint == nullptr);
ctxt->warn (make_unique<concrete_buffer_under_read> (*this, reg,
diag_arg,
- out));
+ bits_outside));
oob_safe = false;
break;
case DIR_WRITE:
ctxt->warn (make_unique<concrete_buffer_underwrite> (*this,
reg, diag_arg,
- out,
+ bits_outside,
sval_hint));
oob_safe = false;
break;
@@ -1030,15 +1259,15 @@ region_model::check_region_bounds (const region *reg,
/* For accesses past the end, we do need a concrete capacity. No need to
do a symbolic check here because the inequality check does not reason
whether constants are greater than symbolic values. */
- if (!cst_capacity_tree)
- return oob_safe;
+ if (!cst_byte_capacity_tree)
+ return oob_safe;
- byte_range buffer (0, wi::to_offset (cst_capacity_tree));
- /* If READ_BYTES exceeds BUFFER, we do have an overflow. */
- if (read_bytes.exceeds_p (buffer, &out))
+ bit_range buffer (0, wi::to_offset (cst_byte_capacity_tree) * BITS_PER_UNIT);
+ /* If READ_BITS exceeds BUFFER, we do have an overflow. */
+ if (read_bits.exceeds_p (buffer, &bits_outside))
{
- tree byte_bound = wide_int_to_tree (size_type_node,
- buffer.get_next_byte_offset ());
+ tree bit_bound = wide_int_to_tree (size_type_node,
+ buffer.get_next_bit_offset ());
tree diag_arg = get_representative_tree (base_reg);
switch (dir)
@@ -1050,13 +1279,15 @@ region_model::check_region_bounds (const region *reg,
gcc_assert (sval_hint == nullptr);
ctxt->warn (make_unique<concrete_buffer_over_read> (*this,
reg, diag_arg,
- out, byte_bound));
+ bits_outside,
+ bit_bound));
oob_safe = false;
break;
case DIR_WRITE:
ctxt->warn (make_unique<concrete_buffer_overflow> (*this,
reg, diag_arg,
- out, byte_bound,
+ bits_outside,
+ bit_bound,
sval_hint));
oob_safe = false;
break;
diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 9b27e8febcc9..d58d064f9ff1 100644
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -757,6 +757,24 @@ int_size_in_bits (const_tree type, bit_size_t *out)
return false;
}
+/* Base implementation of region::get_bit_size_sval vfunc. */
+
+const svalue *
+region::get_bit_size_sval (region_model_manager *mgr) const
+{
+ tree type = get_type ();
+
+ /* Bail out e.g. for heap-allocated regions. */
+ if (!type)
+ return mgr->get_or_create_unknown_svalue (size_type_node);
+
+ bit_size_t bits;
+ if (!int_size_in_bits (type, &bits))
+ return mgr->get_or_create_unknown_svalue (size_type_node);
+
+ return mgr->get_or_create_int_cst (size_type_node, bits);
+}
+
/* If the size of this region (in bits) is known statically, write it to *OUT
and return true.
Otherwise return false. */
@@ -1933,6 +1951,15 @@ offset_region::dump_to_pp (pretty_printer *pp, bool
simple) const
}
}
+const svalue *
+offset_region::get_bit_offset (region_model_manager *mgr) const
+{
+ const svalue *bits_per_byte_sval
+ = mgr->get_or_create_int_cst (size_type_node, BITS_PER_UNIT);
+ return mgr->get_or_create_binop (size_type_node, MULT_EXPR,
+ m_byte_offset, bits_per_byte_sval);
+}
+
/* Implementation of region::get_relative_concrete_offset vfunc
for offset_region. */
@@ -1987,6 +2014,30 @@ offset_region::get_byte_size_sval (region_model_manager
*mgr) const
return region::get_byte_size_sval (mgr);
}
+/* Implementation of region::get_bit_size_sval vfunc for offset_region. */
+
+const svalue *
+offset_region::get_bit_size_sval (region_model_manager *mgr) const
+{
+ tree offset_cst = get_bit_offset (mgr)->maybe_get_constant ();
+ bit_size_t bit_size;
+ /* If the offset points in the middle of the region,
+ return the remaining bits. */
+ if (get_bit_size (&bit_size) && offset_cst)
+ {
+ bit_size_t offset = wi::to_offset (offset_cst);
+ bit_range r (0, bit_size);
+ if (r.contains_p (offset))
+ {
+ tree remaining_bit_size = wide_int_to_tree (size_type_node,
+ bit_size - offset);
+ return mgr->get_or_create_constant_svalue (remaining_bit_size);
+ }
+ }
+
+ return region::get_bit_size_sval (mgr);
+}
+
/* class sized_region : public region. */
/* Implementation of region::accept vfunc for sized_region. */
@@ -2047,6 +2098,17 @@ sized_region::get_bit_size (bit_size_t *out) const
return true;
}
+/* Implementation of region::get_bit_size_sval vfunc for sized_region. */
+
+const svalue *
+sized_region::get_bit_size_sval (region_model_manager *mgr) const
+{
+ const svalue *bits_per_byte_sval
+ = mgr->get_or_create_int_cst (size_type_node, BITS_PER_UNIT);
+ return mgr->get_or_create_binop (size_type_node, MULT_EXPR,
+ m_byte_size_sval, bits_per_byte_sval);
+}
+
/* class cast_region : public region. */
/* Implementation of region::accept vfunc for cast_region. */
@@ -2198,6 +2260,15 @@ bit_range_region::get_byte_size_sval
(region_model_manager *mgr) const
return mgr->get_or_create_int_cst (size_type_node, num_bytes);
}
+/* Implementation of region::get_bit_size_sval vfunc for bit_range_region. */
+
+const svalue *
+bit_range_region::get_bit_size_sval (region_model_manager *mgr) const
+{
+ return mgr->get_or_create_int_cst (size_type_node,
+ m_bits.m_size_in_bits);
+}
+
/* Implementation of region::get_relative_concrete_offset vfunc for
bit_range_region. */
diff --git a/gcc/analyzer/region.h b/gcc/analyzer/region.h
index fee161cf8630..6b542094064c 100644
--- a/gcc/analyzer/region.h
+++ b/gcc/analyzer/region.h
@@ -199,6 +199,10 @@ public:
(which could be "unknown"). */
virtual const svalue *get_byte_size_sval (region_model_manager *mgr) const;
+ /* Get a symbolic value describing the size of this region in bits
+ (which could be "unknown"). */
+ virtual const svalue *get_bit_size_sval (region_model_manager *mgr) const;
+
/* Attempt to get the offset in bits of this region relative to its parent.
If successful, return true and write to *OUT.
Otherwise return false. */
@@ -969,13 +973,15 @@ public:
void dump_to_pp (pretty_printer *pp, bool simple) const final override;
const svalue *get_byte_offset () const { return m_byte_offset; }
+ const svalue *get_bit_offset (region_model_manager *mgr) const;
bool get_relative_concrete_offset (bit_offset_t *out) const final override;
const svalue *get_relative_symbolic_offset (region_model_manager *mgr)
const final override;
const svalue * get_byte_size_sval (region_model_manager *mgr)
const final override;
-
+ const svalue * get_bit_size_sval (region_model_manager *mgr)
+ const final override;
private:
const svalue *m_byte_offset;
@@ -1070,6 +1076,9 @@ public:
return m_byte_size_sval;
}
+ const svalue *
+ get_bit_size_sval (region_model_manager *) const final override;
+
private:
const svalue *m_byte_size_sval;
};
@@ -1304,6 +1313,7 @@ public:
bool get_byte_size (byte_size_t *out) const final override;
bool get_bit_size (bit_size_t *out) const final override;
const svalue *get_byte_size_sval (region_model_manager *mgr) const final
override;
+ const svalue *get_bit_size_sval (region_model_manager *mgr) const final
override;
bool get_relative_concrete_offset (bit_offset_t *out) const final override;
const svalue *get_relative_symbolic_offset (region_model_manager *mgr)
const final override;
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index be1802e1e5a8..32cb1d968b8a 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -285,6 +285,77 @@ bit_range::intersects_p (const bit_range &other,
return false;
}
+/* Return true if THIS and OTHER intersect and write the number
+ of bits both buffers overlap to *OUT_NUM_OVERLAP_BITS.
+
+ Otherwise return false. */
+
+bool
+bit_range::intersects_p (const bit_range &other,
+ bit_size_t *out_num_overlap_bits) const
+{
+ if (get_start_bit_offset () < other.get_next_bit_offset ()
+ && other.get_start_bit_offset () < get_next_bit_offset ())
+ {
+ bit_offset_t overlap_start = MAX (get_start_bit_offset (),
+ other.get_start_bit_offset ());
+ bit_offset_t overlap_next = MIN (get_next_bit_offset (),
+ other.get_next_bit_offset ());
+ gcc_assert (overlap_next > overlap_start);
+ *out_num_overlap_bits = overlap_next - overlap_start;
+ return true;
+ }
+ else
+ return false;
+}
+
+/* Return true if THIS exceeds OTHER and write the overhanging
+ bit range to OUT_OVERHANGING_BIT_RANGE. */
+
+bool
+bit_range::exceeds_p (const bit_range &other,
+ bit_range *out_overhanging_bit_range) const
+{
+ gcc_assert (!empty_p ());
+
+ if (other.get_next_bit_offset () < get_next_bit_offset ())
+ {
+ /* THIS definitely exceeds OTHER. */
+ bit_offset_t start = MAX (get_start_bit_offset (),
+ other.get_next_bit_offset ());
+ bit_offset_t size = get_next_bit_offset () - start;
+ gcc_assert (size > 0);
+ out_overhanging_bit_range->m_start_bit_offset = start;
+ out_overhanging_bit_range->m_size_in_bits = size;
+ return true;
+ }
+ else
+ return false;
+}
+
+/* Return true if THIS falls short of OFFSET and write the
+ bit range fallen short to OUT_FALL_SHORT_BITS. */
+
+bool
+bit_range::falls_short_of_p (bit_offset_t offset,
+ bit_range *out_fall_short_bits) const
+{
+ gcc_assert (!empty_p ());
+
+ if (get_start_bit_offset () < offset)
+ {
+ /* THIS falls short of OFFSET. */
+ bit_offset_t start = get_start_bit_offset ();
+ bit_offset_t size = MIN (offset, get_next_bit_offset ()) - start;
+ gcc_assert (size > 0);
+ out_fall_short_bits->m_start_bit_offset = start;
+ out_fall_short_bits->m_size_in_bits = size;
+ return true;
+ }
+ else
+ return false;
+}
+
int
bit_range::cmp (const bit_range &br1, const bit_range &br2)
{
@@ -431,77 +502,6 @@ byte_range::contains_p (const byte_range &other,
byte_range *out) const
return false;
}
-/* Return true if THIS and OTHER intersect and write the number
- of bytes both buffers overlap to *OUT_NUM_OVERLAP_BYTES.
-
- Otherwise return false. */
-
-bool
-byte_range::intersects_p (const byte_range &other,
- byte_size_t *out_num_overlap_bytes) const
-{
- if (get_start_byte_offset () < other.get_next_byte_offset ()
- && other.get_start_byte_offset () < get_next_byte_offset ())
- {
- byte_offset_t overlap_start = MAX (get_start_byte_offset (),
- other.get_start_byte_offset ());
- byte_offset_t overlap_next = MIN (get_next_byte_offset (),
- other.get_next_byte_offset ());
- gcc_assert (overlap_next > overlap_start);
- *out_num_overlap_bytes = overlap_next - overlap_start;
- return true;
- }
- else
- return false;
-}
-
-/* Return true if THIS exceeds OTHER and write the overhanging
- byte range to OUT_OVERHANGING_BYTE_RANGE. */
-
-bool
-byte_range::exceeds_p (const byte_range &other,
- byte_range *out_overhanging_byte_range) const
-{
- gcc_assert (!empty_p ());
-
- if (other.get_next_byte_offset () < get_next_byte_offset ())
- {
- /* THIS definitely exceeds OTHER. */
- byte_offset_t start = MAX (get_start_byte_offset (),
- other.get_next_byte_offset ());
- byte_offset_t size = get_next_byte_offset () - start;
- gcc_assert (size > 0);
- out_overhanging_byte_range->m_start_byte_offset = start;
- out_overhanging_byte_range->m_size_in_bytes = size;
- return true;
- }
- else
- return false;
-}
-
-/* Return true if THIS falls short of OFFSET and write the
- byte range fallen short to OUT_FALL_SHORT_BYTES. */
-
-bool
-byte_range::falls_short_of_p (byte_offset_t offset,
- byte_range *out_fall_short_bytes) const
-{
- gcc_assert (!empty_p ());
-
- if (get_start_byte_offset () < offset)
- {
- /* THIS falls short of OFFSET. */
- byte_offset_t start = get_start_byte_offset ();
- byte_offset_t size = MIN (offset, get_next_byte_offset ()) - start;
- gcc_assert (size > 0);
- out_fall_short_bytes->m_start_byte_offset = start;
- out_fall_short_bytes->m_size_in_bytes = size;
- return true;
- }
- else
- return false;
-}
-
/* qsort comparator for byte ranges. */
int
diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h
index d75d69d0b7f4..da5c8b6ffaec 100644
--- a/gcc/analyzer/store.h
+++ b/gcc/analyzer/store.h
@@ -275,10 +275,18 @@ struct bit_range
return (get_start_bit_offset () < other.get_next_bit_offset ()
&& other.get_start_bit_offset () < get_next_bit_offset ());
}
+ bool intersects_p (const bit_range &other,
+ bit_size_t *out_num_overlap_bits) const;
bool intersects_p (const bit_range &other,
bit_range *out_this,
bit_range *out_other) const;
+ bool exceeds_p (const bit_range &other,
+ bit_range *out_overhanging_bit_range) const;
+
+ bool falls_short_of_p (bit_offset_t offset,
+ bit_range *out_fall_short_bits) const;
+
static int cmp (const bit_range &br1, const bit_range &br2);
bit_range operator- (bit_offset_t offset) const;
@@ -321,15 +329,6 @@ struct byte_range
&& m_size_in_bytes == other.m_size_in_bytes);
}
- bool intersects_p (const byte_range &other,
- byte_size_t *out_num_overlap_bytes) const;
-
- bool exceeds_p (const byte_range &other,
- byte_range *out_overhanging_byte_range) const;
-
- bool falls_short_of_p (byte_offset_t offset,
- byte_range *out_fall_short_bytes) const;
-
byte_offset_t get_start_byte_offset () const
{
return m_start_byte_offset;
diff --git a/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr112792.c
b/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr112792.c
new file mode 100644
index 000000000000..0bc7413ce5dd
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/out-of-bounds-pr112792.c
@@ -0,0 +1,18 @@
+/* PR analyzer/112792 ("-Wanalyzer-out-of-bounds false positives seen on
+ Linux kernel with certain unions"). */
+
+typedef unsigned int u32;
+
+union msix_perm {
+ struct {
+ u32 rsvd2 : 8;
+ u32 pasid : 20;
+ };
+ u32 bits;
+} __attribute__((__packed__));
+
+union msix_perm mperm;
+
+void idxd_device_set_perm_entry(u32 pasid) {
+ mperm.pasid = pasid; /* { dg-bogus "buffer-overflow" } */
+}
--
2.26.3
