From ae5a0be4fbbfb8faea9d1bd0656e57f0e3b03bee Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 15 May 2020 11:17:19 -0400 Subject: [PATCH 137/179] FIXME: memset improvements --- gcc/analyzer/analyzer.h | 1 + gcc/analyzer/region-model2.cc | 94 ++++++++++++++++++-- gcc/analyzer/region-model2.h | 3 + gcc/analyzer/store2.cc | 25 ++++++ gcc/analyzer/store2.h | 2 + gcc/testsuite/gcc.dg/analyzer/memset-1.c | 105 +++++++++++++++++++++++ 6 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/memset-1.c diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index 36464312d30..6d37f6264b0 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -152,6 +152,7 @@ extern const char *poison_kind_to_str (enum poison_kind); typedef offset_int bit_offset_t; typedef offset_int bit_size_t; +typedef offset_int byte_size_t; /* The location of a region expressesd as an offset relative to a base region. */ diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index e7b10f9f6cb..4723d8577f4 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -893,7 +893,7 @@ region2::get_offset () const /* FIXME. */ bool -region2::get_bit_size (bit_size_t *out) const +region2::get_byte_size (byte_size_t *out) const { tree type = get_type (); @@ -909,7 +909,7 @@ region2::get_bit_size (bit_size_t *out) const TODO: also, what happens for the "empty array final field" idiom? */ return false; } - *out = bytes * BITS_PER_UNIT; + *out = bytes; return true; /* FIXME: or maybe max_int_size_in_bytes (const_tree type) FIXME: what about varying sizes??? */ @@ -921,6 +921,18 @@ region2::get_bit_size (bit_size_t *out) const /* FIXME. */ +bool +region2::get_bit_size (bit_size_t *out) const +{ + byte_size_t byte_size; + if (!get_byte_size (&byte_size)) + return false; + *out = byte_size * BITS_PER_UNIT; + return true; +} + +/* FIXME. */ + region_offset region2::calc_offset () const { @@ -3981,6 +3993,9 @@ public: return m_model->get_rvalue (arg, m_ctxt); } + void dump_to_pp (pretty_printer *pp, bool simple) const; + void dump (bool simple) const; + private: const gcall *m_call; region_model2 *m_model; @@ -3989,6 +4004,33 @@ private: const region2 *m_lhs_region; }; +/* FIXME. */ + +DEBUG_FUNCTION void +call_details::dump_to_pp (pretty_printer *pp, bool simple) const +{ + for (unsigned i = 0; i < gimple_call_num_args (m_call); i++) + { + const svalue2 *arg_sval = get_arg_svalue2 (i); + pp_printf (pp, "arg %i: ", i); + arg_sval->dump_to_pp (pp, simple); + pp_newline (pp); + } +} + +/* FIXME. */ + +DEBUG_FUNCTION void +call_details::dump (bool simple) const +{ + pretty_printer pp; + pp_format_decoder (&pp) = default_tree_printer; + pp_show_color (&pp) = pp_show_color (global_dc->printer); + pp.buffer->stream = stderr; + dump_to_pp (&pp, simple); + pp_flush (&pp); +} + /* Update this model for the CALL stmt, using CTXT to report any diagnostics - the first half. @@ -4211,11 +4253,41 @@ region_model2::impl_call_malloc (const call_details &cd) bool region_model2::impl_call_memset (const call_details &cd) { - const svalue2 *dest = cd.get_arg_svalue2 (0); - const svalue2 *fill_value = cd.get_arg_svalue2 (1); - const svalue2 *num_bytes = cd.get_arg_svalue2 (2); - // TODO - //gcc_unreachable (); + const svalue2 *dest_sval = cd.get_arg_svalue2 (0); + const svalue2 *fill_value_sval = cd.get_arg_svalue2 (1); + const svalue2 *num_bytes_sval = cd.get_arg_svalue2 (2); + + const region2 *dest_reg = deref_rvalue (dest_sval, cd.get_ctxt ()); + + if (tree num_bytes = num_bytes_sval->maybe_get_constant ()) + { + /* "memset" of zero size is a no-op. */ + if (zerop (num_bytes)) + return true; + + /* Set with known amount. */ + // TODO: handle dynamically-sized regions + byte_size_t reg_size; + if (dest_reg->get_byte_size (®_size)) + { + /* Check for an exact size match. */ + /* TODO: issue a diagnostic if it's bigger than the region. */ + if (reg_size == wi::to_offset (num_bytes)) + { + if (tree cst = fill_value_sval->maybe_get_constant ()) + { + if (zerop (cst)) + { + zero_fill_region (dest_reg); + return true; + } + } + } + } + } + + /* Otherwise, mark region's contents as unknown. */ + mark_region_as_unknown (dest_reg); return false; } @@ -5395,6 +5467,14 @@ region_model2::zero_fill_region (const region2 *reg) m_store.zero_fill_region (m_mgr->get_store2_manager(), reg); } +/* Mark REG as having unknown content. */ + +void +region_model2::mark_region_as_unknown (const region2 *reg) +{ + m_store.mark_region_as_unknown (m_mgr->get_store2_manager(), reg); +} + /* Determine what is known about the condition "LHS_SVAL OP RHS_SVAL" within this model. */ diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index d54e42720b5..c84999a72a2 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -908,6 +908,7 @@ public: static int cmp_ptrs (const void *, const void *); region_offset get_offset () const; + bool get_byte_size (byte_size_t *out) const; bool get_bit_size (bit_size_t *out) const; virtual path_var get_representative_path_var () const; @@ -1830,6 +1831,8 @@ class region_model2 void clobber_region (const region2 *reg); void purge_region (const region2 *reg); void zero_fill_region (const region2 *reg); + void mark_region_as_unknown (const region2 *reg); + #if 0 const svalue2 *set_to_new_unknown_value (const region2 *dst_reg, tree type, region_model2_context *ctxt); diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc index 812ba8df038..c588edc40f9 100644 --- a/gcc/analyzer/store2.cc +++ b/gcc/analyzer/store2.cc @@ -379,6 +379,21 @@ binding_cluster2::zero_fill_region (store2_manager *mgr, const region2 *reg) /* FIXME. */ +void +binding_cluster2::mark_region_as_unknown (store2_manager *mgr, + const region2 *reg) +{ + remove_overlapping_bindings (mgr, reg); + + /* Add a default binding to "unknown". */ + region_model2_manager *sval_mgr = mgr->get_svalue2_manager (); + const svalue2 *sval + = sval_mgr->get_or_create_unknown_svalue2 (reg->get_type ()); + bind (mgr, reg, sval, BK_default); +} + +/* FIXME. */ + const svalue2 * binding_cluster2::get_binding (store2_manager *mgr, const region2 *reg, @@ -891,6 +906,16 @@ store2::zero_fill_region (store2_manager *mgr, const region2 *reg) /* FIXME. */ +void +store2::mark_region_as_unknown (store2_manager *mgr, const region2 *reg) +{ + const region2 *base_reg = reg->get_base_region (); + binding_cluster2 *cluster = get_or_create_cluster (base_reg); + cluster->mark_region_as_unknown (mgr, reg); +} + +/* FIXME. */ + const binding_cluster2 * store2::get_cluster (const region2 *base_reg) const { diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h index 3ab0f80a0c6..43d148318b6 100644 --- a/gcc/analyzer/store2.h +++ b/gcc/analyzer/store2.h @@ -201,6 +201,7 @@ public: void clobber_region (store2_manager *mgr, const region2 *reg); void purge_region (store2_manager *mgr, const region2 *reg); void zero_fill_region (store2_manager *mgr, const region2 *reg); + void mark_region_as_unknown (store2_manager *mgr, const region2 *reg); const svalue2 *get_binding (store2_manager *mgr, const region2 *reg, binding_kind kind) const; @@ -296,6 +297,7 @@ public: void clobber_region (store2_manager *mgr, const region2 *reg); void purge_region (store2_manager *mgr, const region2 *reg); void zero_fill_region (store2_manager *mgr, const region2 *reg); + void mark_region_as_unknown (store2_manager *mgr, const region2 *reg); const binding_cluster2 *get_cluster (const region2 *base_reg) const; binding_cluster2 *get_cluster (const region2 *base_reg); diff --git a/gcc/testsuite/gcc.dg/analyzer/memset-1.c b/gcc/testsuite/gcc.dg/analyzer/memset-1.c new file mode 100644 index 00000000000..09974bfb90b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memset-1.c @@ -0,0 +1,105 @@ +#include +#include "analyzer-decls.h" + +/* Zero-fill of uninitialized buffer. */ + +void test_1 (void) +{ + char buf[256]; + memset (buf, 0, 256); + __analyzer_eval (buf[42] == 0); /* { dg-warning "TRUE" } */ +} + +/* Zero-fill of partially initialized buffer. */ + +void test_2 (void) +{ + char buf[256]; + buf[42] = 'A'; + __analyzer_eval (buf[42] == 'A'); /* { dg-warning "TRUE" } */ + memset (buf, 0, 256); + __analyzer_eval (buf[42] == '\0'); /* { dg-warning "TRUE" } */ +} + +/* A "memset" with known non-zero value. */ + +void test_3 (int val) +{ + char buf[256]; + memset (buf, 'A', 256); + /* We currently merely mark such regions as "unknown", so querying + values within them yields UNKNOWN when ideally it would be TRUE. */ + __analyzer_eval (buf[42] == 'A'); /* { dg-warning "TRUE" "known nonzero" { xfail *-*-* } } */ + /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */ +} + +/* A "memset" with unknown value. */ + +void test_4 (int val) +{ + char buf[256]; + memset (buf, val, 256); + /* We currently merely mark such regions as "unknown", so querying + values within them yields UNKNOWN when ideally it would be TRUE. */ + __analyzer_eval (buf[42] == (char)val); /* { dg-warning "TRUE" "known nonzero" { xfail *-*-* } } */ + /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */ +} + +/* A "memset" with unknown num bytes. */ + +void test_5 (int n) +{ + char buf[256]; + buf[42] = 'A'; + __analyzer_eval (buf[42] == 'A'); /* { dg-warning "TRUE" } */ + memset (buf, 0, n); + + /* We can't know if buf[42] was written to or not. */ + __analyzer_eval (buf[42] == 'A'); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (buf[42] == '\0'); /* { dg-warning "UNKNOWN" } */ +} + +/* A "memset" with unknown value, but with zero size. */ + +static size_t __attribute__((noinline)) +get_zero (void) +{ + return 0; +} + +void test_6 (int val) +{ + char buf[256]; + buf[42] = 'A'; + memset (buf, 'B', get_zero ()); + __analyzer_eval (buf[42] == 'A'); /* { dg-warning "TRUE" } */ +} + +/* A "memset" of known size that's not the full buffer. */ + +void test_7 (void) +{ + char buf[256]; + buf[128] = 'A'; + memset (buf, 0, 128); + /* We currently merely mark the whole region as "unknown", so querying + values within them yields UNKNOWN. */ + __analyzer_eval (buf[127] == '\0'); /* { dg-warning "TRUE" "known nonzero" { xfail *-*-* } } */ + /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */ + __analyzer_eval (buf[128] == 'A'); /* { dg-warning "TRUE" "known nonzero" { xfail *-*-* } } */ + /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */ +} + +/* +DONE: + zero size + value not zero + overwriting existing value + +TODO: + not the full region + heap region + VLA + NULL pointer + poisoned values + too big (buffer overflow). */ -- 2.21.0