Skip to content
Snippets Groups Projects
Commit 73da34a5 authored by David Malcolm's avatar David Malcolm
Browse files

analyzer: remove default return value from region_model::on_call_pre


Previously, the code for simulating calls to external functions in
region_model::on_call_pre wrote a default svalue to the LHS of the
call statement, which could be further overwritten by known_function
subclasses.

Unfortunately, this led to messy hacks, such as when the default svalue
was an allocation: the LHS would be written to with two different
heap-allocated regions, requiring special-case cleanups to avoid the
stray state from the first heap allocation leading to state explosions;
see r14-3001-g021077b94741c9.

The following patch eliminates this write of a default svalue to the LHS
of callsite.  Instead, all known_function implementations that have a
return value are now responsible for set the LHS themselves.  A new
call_details::set_any_lhs_with_defaults function is provided to make it
easy to get the old behavior.

On working through the various known_function subclasses, I noticed that
memset was using the default behavior.  That patch updates this so that
it's now known to return its first parameter.

Cleaning this up eliminates various doubling of saved_diagnostics (e.g.
for dubious_allocation_size) where it was generating a diagnostic for
both writes to the LHS, deduplicating them to the first diagnostic (with
the default LHS), and then failing to create a region_creation_event
when emitting the diagnostic, leading to the fallback wording in
dubious_allocation_size::describe_final_event, such as:

  (1) allocated 42 bytes and assigned to ‘int32_t *’ {aka ‘int *’} here; ‘sizeof (int32_t {aka int})’ is ‘4’

Without the double write to the LHS, it creates a region_creation_event,
so we get the allocation and the assignment as two separate events in
the diagnostic path, e.g.:

  (1) allocated 42 bytes here
  (2) assigned to ‘int32_t *’ {aka ‘int *’} here; ‘sizeof (int32_t {aka int})’ is ‘4’

gcc/analyzer/ChangeLog:
	* analyzer.h (class pure_known_function_with_default_return): New
	subclass.
	* call-details.cc (const_fn_p): Move here from region-model.cc.
	(maybe_get_const_fn_result): Likewise.
	(get_result_size_in_bytes): Likewise.
	(call_details::set_any_lhs_with_defaults): New function, based on
	code in region_model::on_call_pre.
	* call-details.h (call_details::set_any_lhs_with_defaults): New
	decl.
	* diagnostic-manager.cc
	(diagnostic_manager::emit_saved_diagnostic): Log the index of the
	saved_diagnostic.
	* kf.cc (pure_known_function_with_default_return::impl_call_pre):
	New.
	(kf_memset::impl_call_pre): Set the LHS to the first param.
	(kf_putenv::impl_call_pre): Call cd.set_any_lhs_with_defaults.
	(kf_sprintf::impl_call_pre): Call cd.set_any_lhs_with_defaults.
	(class kf_stack_restore): Derive from
	pure_known_function_with_default_return.
	(class kf_stack_save): Likewise.
	(kf_strlen::impl_call_pre): Call cd.set_any_lhs_with_defaults.
	* region-model-reachability.cc (reachable_regions::handle_sval):
	Remove logic for symbolic regions for pointers.
	* region-model.cc (region_model::canonicalize): Remove purging of
	dynamic extents workaround for surplus values from
	region_model::on_call_pre's default LHS code.
	(const_fn_p): Move to call-details.cc.
	(maybe_get_const_fn_result): Likewise.
	(get_result_size_in_bytes): Likewise.
	(region_model::update_for_nonzero_return): Call
	cd.set_any_lhs_with_defaults.
	(region_model::on_call_pre): Remove the assignment to the LHS of a
	default return value, instead requiring all known_function
	implementations to write to any LHS of the call.  Use
	cd.set_any_lhs_with_defaults on the non-kf paths.
	* sm-fd.cc (kf_socket::outcome_of_socket::update_model): Use
	cd.set_any_lhs_with_defaults when failing to get at fd state.
	(kf_bind::outcome_of_bind::update_model): Likewise.
	(kf_listen::outcome_of_listen::update_model): Likewise.
	(kf_accept::outcome_of_accept::update_model): Likewise.
	(kf_connect::outcome_of_connect::update_model): Likewise.
	(kf_read::impl_call_pre): Use cd.set_any_lhs_with_defaults.
	* sm-file.cc (class kf_stdio_output_fn): Derive from
	pure_known_function_with_default_return.
	(class kf_ferror): Likewise.
	(class kf_fileno): Likewise.
	(kf_fgets::impl_call_pre): Use cd.set_any_lhs_with_defaults.
	(kf_read::impl_call_pre): Likewise.
	(class kf_getc): Derive from
	pure_known_function_with_default_return.
	(class kf_getchar): Likewise.
	* varargs.cc (kf_va_arg::impl_call_pre): Use
	cd.set_any_lhs_with_defaults.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/allocation-size-1.c: Update expected results
	to reflect splitting of allocation size and assignment messages
	from a single event into pairs of events
	* gcc.dg/analyzer/allocation-size-2.c: Likewise.
	* gcc.dg/analyzer/allocation-size-3.c: Likewise.
	* gcc.dg/analyzer/allocation-size-4.c: Likewise.
	* gcc.dg/analyzer/allocation-size-multiline-1.c: Likewise.
	* gcc.dg/analyzer/allocation-size-multiline-2.c: Likewise.
	* gcc.dg/analyzer/allocation-size-multiline-3.c: Likewise.
	* gcc.dg/analyzer/memset-1.c (test_1): Verify that the return
	value is the initial argument.
	* gcc.dg/plugin/analyzer_kernel_plugin.c
	(copy_across_boundary_fn::impl_call_pre): Ensure the LHS is set on
	the "known zero size" case.
	* gcc.dg/plugin/analyzer_known_fns_plugin.c
	(known_function_attempt_to_copy::impl_call_pre): Likewise.

Signed-off-by: default avatarDavid Malcolm <dmalcolm@redhat.com>
parent e5fe7f2f
Loading
Showing
with 266 additions and 214 deletions
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment