]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
gimple UIDs, LTO and -fanalyzer [PR98599]
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 13 Apr 2021 01:13:40 +0000 (21:13 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Tue, 13 Apr 2021 01:13:40 +0000 (21:13 -0400)
gimple.h has this comment for gimple's uid field:

  /* UID of this statement.  This is used by passes that want to
     assign IDs to statements.  It must be assigned and used by each
     pass.  By default it should be assumed to contain garbage.  */
  unsigned uid;

and gimple_set_uid has:

   Please note that this UID property is supposed to be undefined at
   pass boundaries.  This means that a given pass should not assume it
   contains any useful value when the pass starts and thus can set it
   to any value it sees fit.

which suggests that any pass can use the uid field as an arbitrary
scratch space.

PR analyzer/98599 reports a case where this error occurs in LTO mode:
  fatal error: Cgraph edge statement index out of range
on certain inputs with -fanalyzer.

The error occurs in the LTRANS phase after -fanalyzer runs in the
WPA phase.  The analyzer pass writes to the uid fields of all stmts.

The error occurs when LTRANS is streaming callgraph edges back in.
The LTO format uses stmt uids to associate call stmts with callgraph
edges between WPA and LTRANS.
For example, in lto-cgraph.c, lto_output_edge writes out the
gimple_uid, and input_edge reads it back in.

lto_prepare_function_for_streaming has code to renumber the stmt UIDs
when the code is streamed back out, but for some reason this isn't
called for clones:
    307   /* Do body modifications needed for streaming before we fork out
    308      worker processes.  */
    309   FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
    310     if (!node->clone_of && gimple_has_body_p (node->decl))
    311       lto_prepare_function_for_streaming (node);

Hence the combination of -fanalyzer and -flto will fail in LTRANS's
stream-in if any function clones are encountered.

It's not fully clear to me why this isn't done for clones, and what the
correct fix should be to allow arbitrary changes to uids within WPA
passes.

In the meantime, this patch works around the issue by updating the
analyzer to save and restore the UIDs, fixing the error.

gcc/analyzer/ChangeLog:
PR analyzer/98599
* supergraph.cc (saved_uids::make_uid_unique): New.
(saved_uids::restore_uids): New.
(supergraph::supergraph): Replace assignments to stmt->uid with
calls to m_stmt_uids.make_uid_unique.
(supergraph::~supergraph): New.
* supergraph.h (class saved_uids): New.
(supergraph::~supergraph): New decl.
(supergraph::m_stmt_uids): New field.

gcc/testsuite/ChangeLog:
PR analyzer/98599
* gcc.dg/analyzer/pr98599-a.c: New test.
* gcc.dg/analyzer/pr98599-b.c: New test.

gcc/analyzer/supergraph.cc
gcc/analyzer/supergraph.h
gcc/testsuite/gcc.dg/analyzer/pr98599-a.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/pr98599-b.c [new file with mode: 0644]

index 4b934568db62da2a5f389aaee662b00185da88eb..8611d0f8689868847d9d56ec9cae55b82084c307 100644 (file)
@@ -87,6 +87,50 @@ supergraph_call_edge (function *fun, gimple *stmt)
   return edge;
 }
 
+/* class saved_uids.
+
+   In order to ensure consistent results without relying on the ordering
+   of pointer values we assign a uid to each gimple stmt, globally unique
+   across all functions.
+
+   Normally, the stmt uids are a scratch space that each pass can freely
+   assign its own values to.  However, in the case of LTO, the uids are
+   used to associate call stmts with callgraph edges between the WPA phase
+   (where the analyzer runs in LTO mode) and the LTRANS phase; if the
+   analyzer changes them in the WPA phase, it leads to errors when
+   streaming the code back in at LTRANS.
+   lto_prepare_function_for_streaming has code to renumber the stmt UIDs
+   when the code is streamed back out, but for some reason this isn't
+   called for clones.
+
+   Hence, as a workaround, this class has responsibility for tracking
+   the original uids and restoring them once the pass is complete
+   (in the supergraph dtor).  */
+
+/* Give STMT a globally unique uid, storing its original uid so it can
+   later be restored.  */
+
+void
+saved_uids::make_uid_unique (gimple *stmt)
+{
+  unsigned next_uid = m_old_stmt_uids.length ();
+  unsigned old_stmt_uid = stmt->uid;
+  stmt->uid = next_uid;
+  m_old_stmt_uids.safe_push
+    (std::pair<gimple *, unsigned> (stmt, old_stmt_uid));
+}
+
+/* Restore the saved uids of all stmts.  */
+
+void
+saved_uids::restore_uids () const
+{
+  unsigned i;
+  std::pair<gimple *, unsigned> *pair;
+  FOR_EACH_VEC_ELT (m_old_stmt_uids, i, pair)
+    pair->first->uid = pair->second;
+}
+
 /* supergraph's ctor.  Walk the callgraph, building supernodes for each
    CFG basic block, splitting the basic blocks at callsites.  Join
    together the supernodes with interprocedural and intraprocedural
@@ -101,8 +145,6 @@ supergraph::supergraph (logger *logger)
 
   /* First pass: make supernodes (and assign UIDs to the gimple stmts).  */
   {
-    unsigned next_uid = 0;
-
     /* Sort the cgraph_nodes?  */
     cgraph_node *node;
     FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
@@ -127,7 +169,7 @@ supergraph::supergraph (logger *logger)
            {
              gimple *stmt = gsi_stmt (gpi);
              m_stmt_to_node_t.put (stmt, node_for_stmts);
-             stmt->uid = next_uid++;
+             m_stmt_uids.make_uid_unique (stmt);
            }
 
          /* Append statements from BB to the current supernode, splitting
@@ -139,7 +181,7 @@ supergraph::supergraph (logger *logger)
              gimple *stmt = gsi_stmt (gsi);
              node_for_stmts->m_stmts.safe_push (stmt);
              m_stmt_to_node_t.put (stmt, node_for_stmts);
-             stmt->uid = next_uid++;
+             m_stmt_uids.make_uid_unique (stmt);
              if (cgraph_edge *edge = supergraph_call_edge (fun, stmt))
                {
                  m_cgraph_edge_to_caller_prev_node.put(edge, node_for_stmts);
@@ -257,6 +299,13 @@ supergraph::supergraph (logger *logger)
   }
 }
 
+/* supergraph's dtor.  Reset stmt uids.  */
+
+supergraph::~supergraph ()
+{
+  m_stmt_uids.restore_uids ();
+}
+
 /* Dump this graph in .dot format to PP, using DUMP_ARGS.
    Cluster the supernodes by function, then by BB from original CFG.  */
 
index 5d1268e555f034f226b9e092a7a6c7be626040ad..f4090fd5e0eddfd38ac43e33d39afcff2f50105e 100644 (file)
@@ -79,6 +79,18 @@ struct supergraph_traits
   typedef supercluster cluster_t;
 };
 
+/* A class to manage the setting and restoring of statement uids.  */
+
+class saved_uids
+{
+public:
+  void make_uid_unique (gimple *stmt);
+  void restore_uids () const;
+
+private:
+  auto_vec<std::pair<gimple *, unsigned> > m_old_stmt_uids;
+};
+
 /* A "supergraph" is a directed graph formed by joining together all CFGs,
    linking them via interprocedural call and return edges.
 
@@ -90,6 +102,7 @@ class supergraph : public digraph<supergraph_traits>
 {
 public:
   supergraph (logger *logger);
+  ~supergraph ();
 
   supernode *get_node_for_function_entry (function *fun) const
   {
@@ -205,6 +218,8 @@ private:
 
   typedef hash_map<const function *, unsigned> function_to_num_snodes_t;
   function_to_num_snodes_t m_function_to_num_snodes;
+
+  saved_uids m_stmt_uids;
 };
 
 /* A node within a supergraph.  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98599-a.c b/gcc/testsuite/gcc.dg/analyzer/pr98599-a.c
new file mode 100644 (file)
index 0000000..2bbf37b
--- /dev/null
@@ -0,0 +1,8 @@
+/* { dg-do link } */
+/* { dg-require-effective-target lto } */
+/* { dg-additional-options "-Os -flto" } */
+/* { dg-additional-sources pr98599-b.c } */
+
+int b(int x);
+int a() { b(5); }
+int main() { a(); }
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98599-b.c b/gcc/testsuite/gcc.dg/analyzer/pr98599-b.c
new file mode 100644 (file)
index 0000000..cfdeb3b
--- /dev/null
@@ -0,0 +1 @@
+int b(int x) { return x; }