analyzer: unify bounds-checking class hierarchies
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 1 Dec 2022 02:26:43 +0000 (21:26 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Thu, 1 Dec 2022 02:26:43 +0000 (21:26 -0500)
commit8bc9e4ee874ea3618780413b79b51412dcc40363
tree6489e717d4d466371f77188fd65e566a5ed916bc
parent1d86af242bc4a8e68aebf1f3b8c985f2d17fa791
analyzer: unify bounds-checking class hierarchies

Convert out-of-bounds class hierarchy from:

  pending_diagnostic
    out_of_bounds
      past_the_end
        buffer_overflow (*)
        buffer_over_read (*)
      buffer_underwrite (*)
      buffer_under_read (*)
    symbolic_past_the_end
      symbolic_buffer_overflow (*)
      symbolic_buffer_over_read (*)

to:

  pending_diagnostic
    out_of_bounds
      concrete_out_of_bounds
        concrete_past_the_end
          concrete_buffer_overflow (*)
          concrete_buffer_over_read (*)
        concrete_buffer_underwrite (*)
        concrete_buffer_under_read (*)
      symbolic_past_the_end
        symbolic_buffer_overflow (*)
        symbolic_buffer_over_read (*)

where the concrete classes (i.e. the instantiable ones) are marked
with a (*).

Doing so undercovered a bug where, for CWE-131-examples.c, we were
emitting an extra:
  warning: heap-based buffer over-read [CWE-122] [-Wanalyzer-out-of-bounds]
at the:
  WidgetList[numWidgets] = NULL;
The issue was that within set_next_state we get the rvalue for the LHS,
which looks like a read to the bounds-checker.  The patch fixes this by
passing NULL as the region_model_context * for such accesses.

gcc/analyzer/ChangeLog:
* bounds-checking.cc (class out_of_bounds): Split out from...
(class concrete_out_of_bounds): New abstract subclass.
(class past_the_end): Rename to...
(class concrete_past_the_end): ...this, and make a subclass of
concrete_out_of_bounds.
(class buffer_overflow): Rename to...
(class concrete_buffer_overflow): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_over_read): Rename to...
(class concrete_buffer_over_read): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_underwrite): Rename to...
(class concrete_buffer_underwrite): ...this, and make a subclass
of concrete_out_of_bounds.
(class buffer_under_read): Rename to...
(class concrete_buffer_under_read): ...this, and make a subclass
of concrete_out_of_bounds.
(class symbolic_past_the_end): Convert to a subclass of
out_of_bounds.
(symbolic_buffer_overflow::get_kind): New.
(symbolic_buffer_over_read::get_kind): New.
(region_model::check_region_bounds): Update for renamings.
* engine.cc (impl_sm_context::set_next_state): Eliminate
"new_ctxt", passing NULL to get_rvalue instead.
(impl_sm_context::warn): Likewise.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
gcc/analyzer/bounds-checking.cc
gcc/analyzer/engine.cc