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

Message ID 20221206183800.4095931-1-dmalcolm@redhat.com
State Committed
Commit dcfc7ac94dbcf6c86c0c58ce6dc1d8bd853e4093
Headers
Series [committed] analyzer: use __attribute__((nonnull)) at top level of analysis [PR106325] |

Commit Message

David Malcolm Dec. 6, 2022, 6:38 p.m. UTC
  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.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-4520-gdcfc7ac94dbcf6.

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>
---
 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.dg/analyzer/attribute-nonnull.c       |  18 ++
 6 files changed, 308 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-nonnull-pr106325.c
  

Patch

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" } */
+}