analyzer: use __attribute__((nonnull)) at top level of analysis [PR106325]

PR analyzer/106325 reports false postives from
-Wanalyzer-null-dereference on code like this:

__attribute__((nonnull))
void foo_a (Foo *p)
{
  foo_b (p);

  switch (p->type)
    {
      /* ... */
    }
}

where foo_b (p) has a:

  g_return_if_fail (p);

that expands to:

  if (!p)
    {
      return;
    }

The analyzer "sees" the comparison against NULL in foo_b, and splits the
analysis into the NULL and not-NULL cases; later, back in foo_a,  at
  switch (p->type)
it complains that p is NULL.

Previously we were only using __attribute__((nonnull)) as something to
complain about when it was violated; we weren't using it as a source of
knowledge.

This patch fixes things by making the analyzer respect
__attribute__((nonnull)) at the top-level of the analysis: any such
params are now assumed to be non-NULL, so that the analyzer assumes the
g_return_if_fail inside foo_b doesn't fail when called from foo_a

Doing so fixes the false positives.

gcc/analyzer/ChangeLog:
	PR analyzer/106325
	* region-model-manager.cc
	(region_model_manager::get_or_create_null_ptr): New.
	* region-model-manager.h
	(region_model_manager::get_or_create_null_ptr): New decl.
	* region-model.cc (region_model::on_top_level_param): Add
	"nonnull" param and make use of it.
	(region_model::push_frame): When handling a top-level entrypoint
	to the analysis, determine which params __attribute__((nonnull))
	applies to, and pass to on_top_level_param.
	* region-model.h (region_model::on_top_level_param): Add "nonnull"
	param.

gcc/testsuite/ChangeLog:
	PR analyzer/106325
	* gcc.dg/analyzer/attr-nonnull-pr106325.c: New test.
	* gcc.dg/analyzer/attribute-nonnull.c (test_6): New.
	(test_7): New.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
This commit is contained in:
David Malcolm 2022-12-06 13:26:57 -05:00
parent fa19bfbb0a
commit dcfc7ac94d
6 changed files with 308 additions and 5 deletions

View File

@ -237,6 +237,17 @@ region_model_manager::get_or_create_int_cst (tree type, poly_int64 val)
return get_or_create_constant_svalue (tree_cst);
}
/* Return the svalue * for the constant_svalue for the NULL pointer
of POINTER_TYPE, creating it if necessary. */
const svalue *
region_model_manager::get_or_create_null_ptr (tree pointer_type)
{
gcc_assert (pointer_type);
gcc_assert (POINTER_TYPE_P (pointer_type));
return get_or_create_int_cst (pointer_type, 0);
}
/* Return the svalue * for a unknown_svalue for TYPE (which can be NULL),
creating it if necessary.
The unknown_svalue instances are reused, based on pointer equality

View File

@ -43,6 +43,7 @@ public:
/* svalue consolidation. */
const svalue *get_or_create_constant_svalue (tree cst_expr);
const svalue *get_or_create_int_cst (tree type, poly_int64);
const svalue *get_or_create_null_ptr (tree pointer_type);
const svalue *get_or_create_unknown_svalue (tree type);
const svalue *get_or_create_setjmp_svalue (const setjmp_record &r,
tree type);

View File

@ -4385,11 +4385,13 @@ region_model::apply_constraints_for_exception (const gimple *last_stmt,
PARAM has a defined but unknown initial value.
Anything it points to has escaped, since the calling context "knows"
the pointer, and thus calls to unknown functions could read/write into
the region. */
the region.
If NONNULL is true, then assume that PARAM must be non-NULL. */
void
region_model::on_top_level_param (tree param,
region_model_context *ctxt)
bool nonnull,
region_model_context *ctxt)
{
if (POINTER_TYPE_P (TREE_TYPE (param)))
{
@ -4398,6 +4400,12 @@ region_model::on_top_level_param (tree param,
= m_mgr->get_or_create_initial_value (param_reg);
const region *pointee_reg = m_mgr->get_symbolic_region (init_ptr_sval);
m_store.mark_as_escaped (pointee_reg);
if (nonnull)
{
const svalue *null_ptr_sval
= m_mgr->get_or_create_null_ptr (TREE_TYPE (param));
add_constraint (init_ptr_sval, NE_EXPR, null_ptr_sval, ctxt);
}
}
}
@ -4453,14 +4461,27 @@ region_model::push_frame (function *fun, const vec<const svalue *> *arg_svals,
have defined but unknown initial values.
Anything they point to has escaped. */
tree fndecl = fun->decl;
/* Handle "__attribute__((nonnull))". */
tree fntype = TREE_TYPE (fndecl);
bitmap nonnull_args = get_nonnull_args (fntype);
unsigned parm_idx = 0;
for (tree iter_parm = DECL_ARGUMENTS (fndecl); iter_parm;
iter_parm = DECL_CHAIN (iter_parm))
{
bool non_null = (nonnull_args
? (bitmap_empty_p (nonnull_args)
|| bitmap_bit_p (nonnull_args, parm_idx))
: false);
if (tree parm_default_ssa = ssa_default_def (fun, iter_parm))
on_top_level_param (parm_default_ssa, ctxt);
on_top_level_param (parm_default_ssa, non_null, ctxt);
else
on_top_level_param (iter_parm, ctxt);
on_top_level_param (iter_parm, non_null, ctxt);
parm_idx++;
}
BITMAP_FREE (nonnull_args);
}
return m_current_frame;

View File

@ -530,7 +530,9 @@ private:
int poison_any_pointers_to_descendents (const region *reg,
enum poison_kind pkind);
void on_top_level_param (tree param, region_model_context *ctxt);
void on_top_level_param (tree param,
bool nonnull,
region_model_context *ctxt);
bool called_from_main_p () const;
const svalue *get_initial_value_for_global (const region *reg) const;

View File

@ -0,0 +1,250 @@
typedef long int signed_frame_t;
typedef struct Track Track;
typedef struct ZRegion ZRegion;
typedef struct AutomationTrack AutomationTrack;
typedef struct MidiNote MidiNote;
typedef struct ArrangerObject ArrangerObject;
typedef struct Project Project;
typedef struct ZRegion ZRegion;
typedef struct Position Position;
typedef struct Track Track;
typedef struct ClipEditor ClipEditor;
typedef enum ArrangerObjectType
{
/* .... */
ARRANGER_OBJECT_TYPE_REGION,
ARRANGER_OBJECT_TYPE_MIDI_NOTE,
/* .... */
} ArrangerObjectType;
typedef enum ArrangerObjectPositionType
{
ARRANGER_OBJECT_POSITION_TYPE_START,
ARRANGER_OBJECT_POSITION_TYPE_END,
ARRANGER_OBJECT_POSITION_TYPE_CLIP_START,
ARRANGER_OBJECT_POSITION_TYPE_LOOP_START,
ARRANGER_OBJECT_POSITION_TYPE_LOOP_END,
ARRANGER_OBJECT_POSITION_TYPE_FADE_IN,
ARRANGER_OBJECT_POSITION_TYPE_FADE_OUT,
} ArrangerObjectPositionType;
typedef struct Position
{
/* .... */
double ticks;
/* .... */
} Position;
typedef enum RegionType
{
/* .... */
REGION_TYPE_AUTOMATION = 1 << 2,
/* .... */
} RegionType;
typedef struct RegionIdentifier
{
/* .... */
RegionType type;
/* .... */
int lane_pos;
/* .... */
} RegionIdentifier;
typedef struct ArrangerObject
{
/* .... */
ArrangerObjectType type;
/* .... */
Position pos;
Position end_pos;
Position clip_start_pos;
Position loop_start_pos;
Position loop_end_pos;
Position fade_in_pos;
Position fade_out_pos;
/* .... */
} ArrangerObject;
typedef struct ZRegion
{
/* .... */
RegionIdentifier id;
/* .... */
int num_midi_notes;
/* .... */
} ZRegion;
typedef struct Zrythm
{
/* ... */
Project *project;
/* ... */
} Zrythm;
typedef struct Project
{
/* ... */
ClipEditor *clip_editor;
/* ... */
} Project;
extern Zrythm *zrythm;
extern void g_return_if_fail_warning (const char *log_domain,
const char *pretty_function,
const char *expression);
extern void position_add_ticks (Position *self, double ticks);
extern _Bool
arranger_object_is_position_valid (const ArrangerObject *const self,
const Position *pos,
ArrangerObjectPositionType pos_type);
extern Track *arranger_object_get_track (const ArrangerObject *const self);
extern void midi_region_insert_midi_note (ZRegion *region, MidiNote *midi_note,
int idx, int pub_events);
extern ZRegion *midi_note_get_region (MidiNote *self);
extern AutomationTrack *
region_get_automation_track (const ZRegion *const region);
extern void track_add_region (Track *track, ZRegion *region,
AutomationTrack *at, int lane_pos, int gen_name,
int fire_events);
extern void clip_editor_set_region (ClipEditor *self, ZRegion *region,
_Bool fire_events);
extern ZRegion *clip_editor_get_region (ClipEditor *self);
static Position *
get_position_ptr (ArrangerObject *self, ArrangerObjectPositionType pos_type)
{
switch (pos_type)
{
case ARRANGER_OBJECT_POSITION_TYPE_START:
return &self->pos;
case ARRANGER_OBJECT_POSITION_TYPE_END:
return &self->end_pos;
case ARRANGER_OBJECT_POSITION_TYPE_CLIP_START:
return &self->clip_start_pos;
case ARRANGER_OBJECT_POSITION_TYPE_LOOP_START:
return &self->loop_start_pos;
case ARRANGER_OBJECT_POSITION_TYPE_LOOP_END:
return &self->loop_end_pos;
case ARRANGER_OBJECT_POSITION_TYPE_FADE_IN:
return &self->fade_in_pos;
case ARRANGER_OBJECT_POSITION_TYPE_FADE_OUT:
return &self->fade_out_pos;
}
return (((void *)0));
}
void
arranger_object_set_position (ArrangerObject *self, const Position *pos,
ArrangerObjectPositionType pos_type,
const int validate)
{
if (!(self && pos))
{
g_return_if_fail_warning ("zrythm", ((const char *)(__func__)),
"self && pos");
return;
}
if (validate && !arranger_object_is_position_valid (self, pos, pos_type))
return;
Position *pos_ptr;
pos_ptr = get_position_ptr (self, pos_type);
if (!pos_ptr)
{
g_return_if_fail_warning ("zrythm", ((const char *)(__func__)),
"pos_ptr");
return;
}
*(pos_ptr) = *(pos);
}
void
arranger_object_end_pos_setter (ArrangerObject *self, const Position *pos)
{
arranger_object_set_position (self, pos, ARRANGER_OBJECT_POSITION_TYPE_END,
1);
}
ArrangerObject *
arranger_object_clone (const ArrangerObject *self)
{
if (!self)
{
g_return_if_fail_warning ("zrythm", ((const char *)(__func__)), "self");
return (((void *)0));
}
/* .... */
return (((void *)0));
}
__attribute__((nonnull(1, 2)))
void
arranger_object_unsplit (ArrangerObject *r1, ArrangerObject *r2,
ArrangerObject **obj, _Bool fire_events)
{
ZRegion *clip_editor_region
= clip_editor_get_region (((zrythm)->project->clip_editor));
_Bool set_clip_editor_region = 0;
if (clip_editor_region == (ZRegion *)r1
|| clip_editor_region == (ZRegion *)r2)
{
set_clip_editor_region = 1;
clip_editor_set_region (((zrythm)->project->clip_editor), ((void *)0),
1);
}
*obj = arranger_object_clone (r1);
arranger_object_end_pos_setter (*obj, &r2->end_pos);
Position fade_out_pos;
*(&fade_out_pos) = *(&r2->end_pos);
position_add_ticks (&fade_out_pos, -r2->pos.ticks);
arranger_object_set_position (*obj, &fade_out_pos,
ARRANGER_OBJECT_POSITION_TYPE_FADE_OUT, 0);
switch (r1->type) /* { dg-bogus "dereference of NULL 'r1'" } */
{
case ARRANGER_OBJECT_TYPE_REGION:
{
ZRegion *r1_region = (ZRegion *)r1;
AutomationTrack *at = ((void *)0);
if (r1_region->id.type == REGION_TYPE_AUTOMATION)
{
at = region_get_automation_track (r1_region);
}
track_add_region (arranger_object_get_track (r1), (ZRegion *)*obj, at,
((ZRegion *)r1)->id.lane_pos, 1, fire_events);
}
break;
case ARRANGER_OBJECT_TYPE_MIDI_NOTE:
{
ZRegion *parent_region = midi_note_get_region (((MidiNote *)r1));
midi_region_insert_midi_note (
parent_region, (MidiNote *)*obj,
((ZRegion *)(parent_region))->num_midi_notes, 1);
}
break;
default:
break;
}
switch (r1->type) /* { dg-bogus "dereference of NULL 'r1'" } */
{
/* .... */
default:
break;
}
/* .... */
}

View File

@ -1,3 +1,5 @@
#include "analyzer-decls.h"
#include <stdlib.h>
extern void foo(void *ptrA, void *ptrB, void *ptrC) /* { dg-message "argument 1 of 'foo' must be non-null" } */
@ -81,3 +83,19 @@ void test_5 (void *q, void *r)
free(p);
}
__attribute__((nonnull(1, 3)))
void test_6 (void *p, void *q, void *r)
{
__analyzer_eval (p != NULL); /* { dg-warning "TRUE" } */
__analyzer_eval (q != NULL); /* { dg-warning "UNKNOWN" } */
__analyzer_eval (r != NULL); /* { dg-warning "TRUE" } */
}
__attribute__((nonnull))
void test_7 (void *p, void *q, void *r)
{
__analyzer_eval (p != NULL); /* { dg-warning "TRUE" } */
__analyzer_eval (q != NULL); /* { dg-warning "TRUE" } */
__analyzer_eval (r != NULL); /* { dg-warning "TRUE" } */
}