analyzer: precision-of-wording for -Wanalyzer-stale-setjmp-buffer
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 12 Nov 2020 02:18:59 +0000 (21:18 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Thu, 12 Nov 2020 02:18:59 +0000 (21:18 -0500)
This patch adds a custom event to paths emitted by
-Wanalyzer-stale-setjmp-buffer highlighting the place where the
pertinent stack frame is popped, and updates the final event in
the path to reference this.

gcc/analyzer/ChangeLog:
* checker-path.h (checker_event::get_id_ptr): New.
* diagnostic-manager.cc (path_builder::path_builder): Add "sd"
param and use it to initialize new field "m_sd".
(path_builder::get_pending_diagnostic): New.
(path_builder::m_sd): New field.
(diagnostic_manager::emit_saved_diagnostic): Pass sd to
path_builder ctor.
(diagnostic_manager::add_events_for_superedge): Call new
maybe_add_custom_events_for_superedge vfunc.
* engine.cc (stale_jmp_buf::stale_jmp_buf): Add "setjmp_point"
param and use it to initialize new field "m_setjmp_point".
Initialize new field "m_stack_pop_event".
(stale_jmp_buf::maybe_add_custom_events_for_superedge): New vfunc
implementation.
(stale_jmp_buf::describe_final_event): New vfunc implementation.
(stale_jmp_buf::m_setjmp_point): New field.
(stale_jmp_buf::m_stack_pop_event): New field.
(exploded_node::on_longjmp): Pass setjmp_point to stale_jmp_buf
ctor.
* pending-diagnostic.h
(pending_diagnostic::maybe_add_custom_events_for_superedge): New
vfunc.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/setjmp-5.c: Update expected path output to show
an event where the pertinent stack frame is popped.  Update
expected message from final event to reference this event.

gcc/analyzer/checker-path.h
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/engine.cc
gcc/analyzer/pending-diagnostic.h
gcc/testsuite/gcc.dg/analyzer/setjmp-5.c

index 7b86d48e983d3c51a9ff295ac93ac75d6a602de5..a00b3cf09eaf0e06e44e815d35a7fc7436eafd31 100644 (file)
@@ -97,6 +97,12 @@ public:
   virtual bool is_function_entry_p () const  { return false; }
   virtual bool is_return_p () const  { return false; }
 
+  /* For use with %@.  */
+  const diagnostic_event_id_t *get_id_ptr () const
+  {
+    return &m_emission_id;
+  }
+
   void dump (pretty_printer *pp) const;
 
  public:
index 93f270f7c2c61b6476e045633b203c56f26cf08f..13f6dd2f341e54bb46663e84e8a7307a7d0563b8 100644 (file)
@@ -161,15 +161,22 @@ class path_builder
 public:
   path_builder (const exploded_graph &eg,
                const exploded_path &epath,
-               const feasibility_problem *problem)
+               const feasibility_problem *problem,
+               const saved_diagnostic &sd)
   : m_eg (eg),
     m_diag_enode (epath.get_final_enode ()),
+    m_sd (sd),
     m_reachability (eg, m_diag_enode),
     m_feasibility_problem (problem)
   {}
 
   const exploded_node *get_diag_node () const { return m_diag_enode; }
 
+  pending_diagnostic *get_pending_diagnostic () const
+  {
+    return m_sd.m_d;
+  }
+
   bool reachable_from_p (const exploded_node *src_enode) const
   {
     return m_reachability.reachable_from_p (src_enode);
@@ -190,6 +197,8 @@ private:
   /* The enode where the diagnostic occurs.  */
   const exploded_node *m_diag_enode;
 
+  const saved_diagnostic &m_sd;
+
   /* Precompute all enodes from which the diagnostic is reachable.  */
   enode_reachability m_reachability;
 
@@ -629,7 +638,7 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
   pretty_printer *pp = global_dc->printer->clone ();
 
   /* Precompute all enodes from which the diagnostic is reachable.  */
-  path_builder pb (eg, epath, sd.get_feasibility_problem ());
+  path_builder pb (eg, epath, sd.get_feasibility_problem (), sd);
 
   /* This is the diagnostic_path subclass that will be built for
      the diagnostic.  */
@@ -1175,6 +1184,11 @@ diagnostic_manager::add_events_for_superedge (const path_builder &pb,
 {
   gcc_assert (eedge.m_sedge);
 
+  /* Give diagnostics an opportunity to override this function.  */
+  pending_diagnostic *pd = pb.get_pending_diagnostic ();
+  if (pd->maybe_add_custom_events_for_superedge (eedge, emission_path))
+    return;
+
   /* Don't add events for insignificant edges at verbosity levels below 3.  */
   if (m_verbosity < 3)
     if (!significant_edge_p (pb, eedge))
index d247ebbc20e64ddf41cc12b6e023aaa5865ddff1..0a8e5b87e01a3d97987563b28a51f6ce07c92bb9 100644 (file)
@@ -1277,8 +1277,10 @@ valid_longjmp_stack_p (const program_point &longjmp_point,
 class stale_jmp_buf : public pending_diagnostic_subclass<dump_path_diagnostic>
 {
 public:
-  stale_jmp_buf (const gcall *setjmp_call, const gcall *longjmp_call)
-  : m_setjmp_call (setjmp_call), m_longjmp_call (longjmp_call)
+  stale_jmp_buf (const gcall *setjmp_call, const gcall *longjmp_call,
+                const program_point &setjmp_point)
+  : m_setjmp_call (setjmp_call), m_longjmp_call (longjmp_call),
+    m_setjmp_point (setjmp_point), m_stack_pop_event (NULL)
   {}
 
   bool emit (rich_location *richloc) FINAL OVERRIDE
@@ -1299,9 +1301,56 @@ public:
            && m_longjmp_call == other.m_longjmp_call);
   }
 
+  bool
+  maybe_add_custom_events_for_superedge (const exploded_edge &eedge,
+                                        checker_path *emission_path)
+    FINAL OVERRIDE
+  {
+    /* Detect exactly when the stack first becomes invalid,
+       and issue an event then.  */
+    if (m_stack_pop_event)
+      return false;
+    const exploded_node *src_node = eedge.m_src;
+    const program_point &src_point = src_node->get_point ();
+    const exploded_node *dst_node = eedge.m_dest;
+    const program_point &dst_point = dst_node->get_point ();
+    if (valid_longjmp_stack_p (src_point, m_setjmp_point)
+       && !valid_longjmp_stack_p (dst_point, m_setjmp_point))
+      {
+       /* Compare with diagnostic_manager::add_events_for_superedge.  */
+       const int src_stack_depth = src_point.get_stack_depth ();
+       m_stack_pop_event = new custom_event
+         (src_point.get_location (),
+          src_point.get_fndecl (),
+          src_stack_depth,
+          "stack frame is popped here, invalidating saved environment");
+       emission_path->add_event (m_stack_pop_event);
+       return false;
+      }
+    return false;
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev)
+  {
+    if (m_stack_pop_event)
+      return ev.formatted_print
+       ("%qs called after enclosing function of %qs returned at %@",
+        get_user_facing_name (m_longjmp_call),
+        get_user_facing_name (m_setjmp_call),
+        m_stack_pop_event->get_id_ptr ());
+    else
+      return ev.formatted_print
+       ("%qs called after enclosing function of %qs has returned",
+        get_user_facing_name (m_longjmp_call),
+        get_user_facing_name (m_setjmp_call));;
+  }
+
+
 private:
   const gcall *m_setjmp_call;
   const gcall *m_longjmp_call;
+  program_point m_setjmp_point;
+  custom_event *m_stack_pop_event;
 };
 
 /* Handle LONGJMP_CALL, a call to longjmp or siglongjmp.
@@ -1344,7 +1393,7 @@ exploded_node::on_longjmp (exploded_graph &eg,
   /* Verify that the setjmp's call_stack hasn't been popped.  */
   if (!valid_longjmp_stack_p (longjmp_point, setjmp_point))
     {
-      ctxt->warn (new stale_jmp_buf (setjmp_call, longjmp_call));
+      ctxt->warn (new stale_jmp_buf (setjmp_call, longjmp_call, setjmp_point));
       return;
     }
 
index b1ff2688bcc7d224c853fe6ad120144ff1630634..ce626f678711fe269802ab9dd26d96694756f2c2 100644 (file)
@@ -246,6 +246,21 @@ class pending_diagnostic
   }
 
   /* End of precision-of-wording vfuncs.  */
+
+  /* Vfunc for extending/overriding creation of the events for an
+     exploded_edge that corresponds to a superedge, allowing for custom
+     events to be created that are pertinent to a particular
+     pending_diagnostic subclass.
+
+     For example, the -Wanalyzer-stale-setjmp-buffer diagnostic adds a
+     custom event showing when the pertinent stack frame is popped
+     (and thus the point at which the jmp_buf becomes invalid).  */
+
+  virtual bool maybe_add_custom_events_for_superedge (const exploded_edge &,
+                                                     checker_path *)
+  {
+    return false;
+  }
 };
 
 /* A template to make it easier to make subclasses of pending_diagnostic.
index bf5b9bfda819e6e94aee86142e60f87f345c35ae..4787fa38032b9196b72f326398a4c562e488734b 100644 (file)
@@ -51,18 +51,25 @@ void outer (void)
            |      |   |
            |      |   (4) 'setjmp' called here
            |
+         'inner': event 5
+           |
+           |   NN | }
+           |      | ^
+           |      | |
+           |      | (5) stack frame is popped here, invalidating saved environment
+           |
     <------+
     |
-  'outer': events 5-6
+  'outer': events 6-7
     |
     |   NN |   inner ();
     |      |   ^~~~~~~~
     |      |   |
-    |      |   (5) returning to 'outer' from 'inner'
+    |      |   (6) returning to 'outer' from 'inner'
     |   NN | 
     |   NN |   longjmp (env, 42);
     |      |   ~~~~~~~~~~~~~~~~~
     |      |   |
-    |      |   (6) here
+    |      |   (7) 'longjmp' called after enclosing function of 'setjmp' returned at (5)
     |
     { dg-end-multiline-output "" } */