Re: [PATCH v3] analyzer: add allocation size checker [PR105900]

[David Malcolm <dmalcolm@redhat.com>]

On Fri, 2022-07-01 at 00:11 +0200, Tim Lange wrote:
> Hi,
> 
> here's the updated patch that should address all the comments from the
> v2.
> 
> - Tim
> 
> This patch adds an checker that warns about code paths in which a
> buffer is
> assigned to a incompatible type, i.e. when the allocated buffer size is
> not a
> multiple of the pointee's size.
> 
> 2022-07-30  Tim Lange  <mail@tim-lange.me>
> 
> gcc/analyzer/ChangeLog:
> 
>         PR analyzer/105900
>         * analyzer.opt: Added Wanalyzer-allocation-size.
>         * checker-path.cc (region_creation_event::get_desc): Added call
> to new
>         virtual function
> pending_diagnostic::describe_region_creation_event.
>         * checker-path.h: Added region_creation_event::get_desc.
>         * diagnostic-manager.cc
> (diagnostic_manager::add_event_on_final_node):
>         New function.
>         * diagnostic-manager.h:
>         Added diagnostic_manager::add_event_on_final_node.
>         * pending-diagnostic.h (struct region_creation): New event_desc
> struct.
>         (pending_diagnostic::describe_region_creation_event): Added
> virtual
>         function to overwrite description of a region creation.
>         * region-model.cc (class dubious_allocation_size): New class.
>         (capacity_compatible_with_type): New helper function.
>         (class size_visitor): New class.
>         (struct_or_union_with_inheritance_p): New helper function.
>         (is_any_cast_p): New helper function.
>         (region_model::check_region_size): New function.
>         (region_model::set_value): Added call to
>         region_model::check_region_size.
>         * region-model.h (class region_model): New function
> check_region_size.
>         * svalue.cc (region_svalue::accept): Changed to post-order
> traversal.
>         (initial_svalue::accept): Likewise.
>         (unaryop_svalue::accept): Likewise.
>         (binop_svalue::accept): Likewise.
>         (sub_svalue::accept): Likewise.
>         (repeated_svalue::accept): Likewise.
>         (bits_within_svalue::accept): Likewise.
>         (widening_svalue::accept): Likewise.
>         (unmergeable_svalue::accept): Likewise.
>         (compound_svalue::accept): Likewise.
>         (conjured_svalue::accept): Likewise.
>         (asm_output_svalue::accept): Likewise.
>         (const_fn_result_svalue::accept): Likewise.
> 
> gcc/ChangeLog:
> 
>         PR analyzer/105900
>         * doc/invoke.texi: Added Wanalyzer-allocation-size.
> 
> gcc/testsuite/ChangeLog:
> 
>         PR analyzer/105900
> * gcc.dg/analyzer/pr96639.c: Changed buffer size to omit warning.
>         * gcc.dg/analyzer/allocation-size-1.c: New test.
>         * gcc.dg/analyzer/allocation-size-2.c: New test.
>         * gcc.dg/analyzer/allocation-size-3.c: New test.
>         * gcc.dg/analyzer/allocation-size-4.c: New test.
>         * gcc.dg/analyzer/allocation-size-5.c: New test.
> 
> Signed-off-by: Tim Lange <mail@tim-lange.me>


Thanks for the v3 patch.

Content-wise, the v3 patch looks ready to me, though there's something
weird with the formatting of the ChangeLog entry for pr96639.c in the
commit message - does the patch pass:
  ./contrib/gcc-changelog/git_check_commit.py HEAD
?  (this script gets run server-side on our git repository, and it
won't let you push a patch unless the script passes)

You didn't specify to what extent you've tested it.  If you've
successfully bootstrapped gcc with this patch applied, and run the test
suite with no regressions, then this is OK to push to trunk.

[...snip...]

Thanks
Dave