From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2209) id D72613875B78; Tue, 6 Dec 2022 18:28:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D72613875B78 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670351298; bh=8vU3J83J5phoU/fo5+/uz798CGhuE17aI1lSygizxIo=; h=From:To:Subject:Date:From; b=SK06IbjLxFXdAbn7/w7NixomPYxaMDkYabB6wEMEyK6FnR1g2I4EGDR/DT1+6qHix iyHNPJEIxN8OLuILCjKWdy+4pjSe0WBWU/RCajOjhn9dz4XIV30HU6bZlA8Owraqu6 0bg/2N9KGCEtrm59wIfrsd9/X/dk+L+N1eRckzoA= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: David Malcolm To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-4520] analyzer: use __attribute__((nonnull)) at top level of analysis [PR106325] X-Act-Checkin: gcc X-Git-Author: David Malcolm X-Git-Refname: refs/heads/master X-Git-Oldrev: fa19bfbb0a19950fcbce8c3ccb0a116faab477d0 X-Git-Newrev: dcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093 Message-Id: <20221206182818.D72613875B78@sourceware.org> Date: Tue, 6 Dec 2022 18:28:18 +0000 (GMT) List-Id: https://gcc.gnu.org/g:dcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093 commit r13-4520-gdcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093 Author: David Malcolm Date: Tue Dec 6 13:26:57 2022 -0500 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 Diff: --- gcc/analyzer/region-model-manager.cc | 11 + gcc/analyzer/region-model-manager.h | 1 + gcc/analyzer/region-model.cc | 29 ++- gcc/analyzer/region-model.h | 4 +- .../gcc.dg/analyzer/attr-nonnull-pr106325.c | 250 +++++++++++++++++++++ gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c | 18 ++ 6 files changed, 308 insertions(+), 5 deletions(-) diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc index 471a9272e41..0fb96386f28 100644 --- a/gcc/analyzer/region-model-manager.cc +++ b/gcc/analyzer/region-model-manager.cc @@ -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 diff --git a/gcc/analyzer/region-model-manager.h b/gcc/analyzer/region-model-manager.h index 22f980056fa..13fbe483f6d 100644 --- a/gcc/analyzer/region-model-manager.h +++ b/gcc/analyzer/region-model-manager.h @@ -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); diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index d00f15f468f..430c0e91175 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -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 *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; diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 3b93d3e16b8..291bb2ff45a 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -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; diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-nonnull-pr106325.c b/gcc/testsuite/gcc.dg/analyzer/attr-nonnull-pr106325.c new file mode 100644 index 00000000000..3b264719f6d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-nonnull-pr106325.c @@ -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; + } + /* .... */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c index 70bb921e742..7c71a71c930 100644 --- a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c +++ b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c @@ -1,3 +1,5 @@ +#include "analyzer-decls.h" + #include 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" } */ +}