public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-4520] analyzer: use __attribute__((nonnull)) at top level of analysis [PR106325]
@ 2022-12-06 18:28 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-12-06 18:28 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:dcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093
commit r13-4520-gdcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093
Author: David Malcolm <dmalcolm@redhat.com>
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 <dmalcolm@redhat.com>
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<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;
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 <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" } */
+}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-12-06 18:28 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 18:28 [gcc r13-4520] analyzer: use __attribute__((nonnull)) at top level of analysis [PR106325] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).