ipa: Careful processing ANCESTOR jump functions and NULL pointers (PR 103083)

IPA_JF_ANCESTOR jump functions are constructed also when the formal
parameter of the caller is first checked whether it is NULL and left
as it is if it is NULL, to accommodate C++ casts to an ancestor class.

The jump function type was invented for devirtualization and IPA-CP
propagation of tree constants is also careful to apply it only to
existing DECLs(*) but as PR 103083 shows, the part propagating "known
bits" was not careful about this, which can lead to miscompilations.

This patch introduces a flag to the ancestor jump functions which
tells whether a NULL-check was elided when creating it and makes the
bits propagation behave accordingly, masking any bits otherwise would
be known to be one.  This should safely preserve alignment info, which
is the primary ifnormation that we keep in bits for pointers.

(*) There still may remain problems when a DECL resides on address
zero (with -fno-delete-null-pointer-checks ...I hope it cannot happen
otherwise).  I am looking into that now but I think it will be easier
for everyone if I do so in a follow-up patch.

gcc/ChangeLog:

2022-02-11  Martin Jambor  <mjambor@suse.cz>

	PR ipa/103083
	* ipa-prop.h (ipa_ancestor_jf_data): New flag keep_null;
	(ipa_get_jf_ancestor_keep_null): New function.
	* ipa-prop.cc (ipa_set_ancestor_jf): Initialize keep_null field of the
	ancestor function.
	(compute_complex_assign_jump_func): Pass false to keep_null
	parameter of ipa_set_ancestor_jf.
	(compute_complex_ancestor_jump_func): Pass true to keep_null
	parameter of ipa_set_ancestor_jf.
	(update_jump_functions_after_inlining): Carry over keep_null from the
	original ancestor jump-function or merge them.
	(ipa_write_jump_function): Stream keep_null flag.
	(ipa_read_jump_function): Likewise.
	(ipa_print_node_jump_functions_for_edge): Print the new flag.
	* ipa-cp.cc (class ipcp_bits_lattice): Make various getters const.  New
	member function known_nonzero_p.
	(ipcp_bits_lattice::known_nonzero_p): New.
	(ipcp_bits_lattice::meet_with_1): New parameter drop_all_ones,
	observe it.
	(ipcp_bits_lattice::meet_with): Likewise.
	(propagate_bits_across_jump_function): Simplify.  Pass true in
	drop_all_ones when it is necessary.
	(propagate_aggs_across_jump_function): Take care of keep_null
	flag.
	(ipa_get_jf_ancestor_result): Propagate NULL accross keep_null
	jump functions.

gcc/testsuite/ChangeLog:

2021-11-25  Martin Jambor  <mjambor@suse.cz>

	* gcc.dg/ipa/pr103083-1.c: New test.
	* gcc.dg/ipa/pr103083-2.c: Likewise.

diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index 8628140..dc3f0e9 100644 (file)
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -306,17 +306,18 @@ public:
 class ipcp_bits_lattice
 {
 public:
-  bool bottom_p () { return m_lattice_val == IPA_BITS_VARYING; }
-  bool top_p () { return m_lattice_val == IPA_BITS_UNDEFINED; }
-  bool constant_p () { return m_lattice_val == IPA_BITS_CONSTANT; }
+  bool bottom_p () const { return m_lattice_val == IPA_BITS_VARYING; }
+  bool top_p () const { return m_lattice_val == IPA_BITS_UNDEFINED; }
+  bool constant_p () const { return m_lattice_val == IPA_BITS_CONSTANT; }
   bool set_to_bottom ();
   bool set_to_constant (widest_int, widest_int);
+  bool known_nonzero_p () const;
 
-  widest_int get_value () { return m_value; }
-  widest_int get_mask () { return m_mask; }
+  widest_int get_value () const { return m_value; }
+  widest_int get_mask () const { return m_mask; }
 
   bool meet_with (ipcp_bits_lattice& other, unsigned, signop,
-                 enum tree_code, tree);
+                 enum tree_code, tree, bool);
 
   bool meet_with (widest_int, widest_int, unsigned);
 
@@ -330,7 +331,7 @@ private:
      value is known to be constant.  */
   widest_int m_value, m_mask;
 
-  bool meet_with_1 (widest_int, widest_int, unsigned);
+  bool meet_with_1 (widest_int, widest_int, unsigned, bool);
   void get_value_and_mask (tree, widest_int *, widest_int *);
 };
 
@@ -1081,6 +1082,16 @@ ipcp_bits_lattice::set_to_constant (widest_int value, widest_int mask)
   return true;
 }
 
+/* Return true if any of the known bits are non-zero.  */
+
+bool
+ipcp_bits_lattice::known_nonzero_p () const
+{
+  if (!constant_p ())
+    return false;
+  return wi::ne_p (wi::bit_and (wi::bit_not (m_mask), m_value), 0);
+}
+
 /* Convert operand to value, mask form.  */
 
 void
@@ -1103,16 +1114,19 @@ ipcp_bits_lattice::get_value_and_mask (tree operand, widest_int *valuep, widest_
 /* Meet operation, similar to ccp_lattice_meet, we xor values
    if this->value, value have different values at same bit positions, we want
    to drop that bit to varying. Return true if mask is changed.
-   This function assumes that the lattice value is in CONSTANT state  */
+   This function assumes that the lattice value is in CONSTANT state.  If
+   DROP_ALL_ONES, mask out any known bits with value one afterwards.  */
 
 bool
 ipcp_bits_lattice::meet_with_1 (widest_int value, widest_int mask,
-                               unsigned precision)
+                               unsigned precision, bool drop_all_ones)
 {
   gcc_assert (constant_p ());
 
   widest_int old_mask = m_mask;
   m_mask = (m_mask | mask) | (m_value ^ value);
+  if (drop_all_ones)
+    m_mask |= m_value;
   m_value &= ~m_mask;
 
   if (wi::sext (m_mask, precision) == -1)
@@ -1138,16 +1152,18 @@ ipcp_bits_lattice::meet_with (widest_int value, widest_int mask,
       return set_to_constant (value, mask);
     }
 
-  return meet_with_1 (value, mask, precision);
+  return meet_with_1 (value, mask, precision, false);
 }
 
 /* Meet bits lattice with the result of bit_value_binop (other, operand)
    if code is binary operation or bit_value_unop (other) if code is unary op.
-   In the case when code is nop_expr, no adjustment is required. */
+   In the case when code is nop_expr, no adjustment is required.  If
+   DROP_ALL_ONES, mask out any known bits with value one afterwards.  */
 
 bool
 ipcp_bits_lattice::meet_with (ipcp_bits_lattice& other, unsigned precision,
-                             signop sgn, enum tree_code code, tree operand)
+                             signop sgn, enum tree_code code, tree operand,
+                             bool drop_all_ones)
 {
   if (other.bottom_p ())
     return set_to_bottom ();
@@ -1186,12 +1202,18 @@ ipcp_bits_lattice::meet_with (ipcp_bits_lattice& other, unsigned precision,
 
   if (top_p ())
     {
+      if (drop_all_ones)
+       {
+         adjusted_mask |= adjusted_value;
+         adjusted_value &= ~adjusted_mask;
+       }
       if (wi::sext (adjusted_mask, precision) == -1)
        return set_to_bottom ();
       return set_to_constant (adjusted_value, adjusted_mask);
     }
   else
-    return meet_with_1 (adjusted_value, adjusted_mask, precision);
+    return meet_with_1 (adjusted_value, adjusted_mask, precision,
+                       drop_all_ones);
 }
 
 /* Mark bot aggregate and scalar lattices as containing an unknown variable,
@@ -1477,6 +1499,9 @@ ipa_get_jf_ancestor_result (struct ipa_jump_func *jfunc, tree input)
                     fold_build2 (MEM_REF, TREE_TYPE (TREE_TYPE (input)), input,
                                  build_int_cst (ptr_type_node, byte_offset)));
     }
+  else if (ipa_get_jf_ancestor_keep_null (jfunc)
+          && zerop (input))
+    return input;
   else
     return NULL_TREE;
 }
@@ -2373,6 +2398,7 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
       tree operand = NULL_TREE;
       enum tree_code code;
       unsigned src_idx;
+      bool keep_null = false;
 
       if (jfunc->type == IPA_JF_PASS_THROUGH)
        {
@@ -2385,7 +2411,9 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
        {
          code = POINTER_PLUS_EXPR;
          src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
-         unsigned HOST_WIDE_INT offset = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
+         unsigned HOST_WIDE_INT offset
+           = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
+         keep_null = (ipa_get_jf_ancestor_keep_null (jfunc) || !offset);
          operand = build_int_cstu (size_type_node, offset);
        }
 
@@ -2402,18 +2430,17 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
         result of x & 0xff == 0xff, which gets computed during ccp1 pass
         and we store it in jump function during analysis stage.  */
 
-      if (src_lats->bits_lattice.bottom_p ()
-         && jfunc->bits)
-       return dest_lattice->meet_with (jfunc->bits->value, jfunc->bits->mask,
-                                       precision);
-      else
-       return dest_lattice->meet_with (src_lats->bits_lattice, precision, sgn,
-                                       code, operand);
+      if (!src_lats->bits_lattice.bottom_p ())
+       {
+         bool drop_all_ones
+           = keep_null && !src_lats->bits_lattice.known_nonzero_p ();
+
+         return dest_lattice->meet_with (src_lats->bits_lattice, precision,
+                                         sgn, code, operand, drop_all_ones);
+       }
     }
 
-  else if (jfunc->type == IPA_JF_ANCESTOR)
-    return dest_lattice->set_to_bottom ();
-  else if (jfunc->bits)
+  if (jfunc->bits)
     return dest_lattice->meet_with (jfunc->bits->value, jfunc->bits->mask,
                                    precision);
   else

diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index 72aa3e2..0e59663 100644 (file)
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -357,6 +357,8 @@ ipa_print_node_jump_functions_for_edge (FILE *f, struct cgraph_edge *cs)
                   jump_func->value.ancestor.offset);
          if (jump_func->value.ancestor.agg_preserved)
            fprintf (f, ", agg_preserved");
+         if (jump_func->value.ancestor.keep_null)
+           fprintf (f, ", keep_null");
          fprintf (f, "\n");
        }
 
@@ -601,12 +603,13 @@ ipa_set_jf_arith_pass_through (struct ipa_jump_func *jfunc, int formal_id,
 
 static void
 ipa_set_ancestor_jf (struct ipa_jump_func *jfunc, HOST_WIDE_INT offset,
-                    int formal_id, bool agg_preserved)
+                    int formal_id, bool agg_preserved, bool keep_null)
 {
   jfunc->type = IPA_JF_ANCESTOR;
   jfunc->value.ancestor.formal_id = formal_id;
   jfunc->value.ancestor.offset = offset;
   jfunc->value.ancestor.agg_preserved = agg_preserved;
+  jfunc->value.ancestor.keep_null = keep_null;
 }
 
 /* Get IPA BB information about the given BB.  FBI is the context of analyzis
@@ -1438,7 +1441,8 @@ compute_complex_assign_jump_func (struct ipa_func_body_info *fbi,
   index = ipa_get_param_decl_index (info, SSA_NAME_VAR (ssa));
   if (index >= 0 && param_type && POINTER_TYPE_P (param_type))
     ipa_set_ancestor_jf (jfunc, offset,  index,
-                        parm_ref_data_pass_through_p (fbi, index, call, ssa));
+                        parm_ref_data_pass_through_p (fbi, index, call, ssa),
+                        false);
 }
 
 /* Extract the base, offset and MEM_REF expression from a statement ASSIGN if
@@ -1564,7 +1568,8 @@ compute_complex_ancestor_jump_func (struct ipa_func_body_info *fbi,
     }
 
   ipa_set_ancestor_jf (jfunc, offset, index,
-                      parm_ref_data_pass_through_p (fbi, index, call, parm));
+                      parm_ref_data_pass_through_p (fbi, index, call, parm),
+                      true);
 }
 
 /* Inspect the given TYPE and return true iff it has the same structure (the
@@ -3250,6 +3255,7 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
              dst->value.ancestor.offset += src->value.ancestor.offset;
              dst->value.ancestor.agg_preserved &=
                src->value.ancestor.agg_preserved;
+             dst->value.ancestor.keep_null |= src->value.ancestor.keep_null;
            }
          else
            ipa_set_jf_unknown (dst);
@@ -3327,7 +3333,8 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
                    ipa_set_ancestor_jf (dst,
                                         ipa_get_jf_ancestor_offset (src),
                                         ipa_get_jf_ancestor_formal_id (src),
-                                        agg_p);
+                                        agg_p,
+                                        ipa_get_jf_ancestor_keep_null (src));
                    break;
                  }
                default:
@@ -4760,6 +4767,7 @@ ipa_write_jump_function (struct output_block *ob,
       streamer_write_uhwi (ob, jump_func->value.ancestor.formal_id);
       bp = bitpack_create (ob->main_stream);
       bp_pack_value (&bp, jump_func->value.ancestor.agg_preserved, 1);
+      bp_pack_value (&bp, jump_func->value.ancestor.keep_null, 1);
       streamer_write_bitpack (&bp);
       break;
     default:
@@ -4885,7 +4893,9 @@ ipa_read_jump_function (class lto_input_block *ib,
        int formal_id = streamer_read_uhwi (ib);
        struct bitpack_d bp = streamer_read_bitpack (ib);
        bool agg_preserved = bp_unpack_value (&bp, 1);
-       ipa_set_ancestor_jf (jump_func, offset, formal_id, agg_preserved);
+       bool keep_null = bp_unpack_value (&bp, 1);
+       ipa_set_ancestor_jf (jump_func, offset, formal_id, agg_preserved,
+                            keep_null);
        break;
       }
     default:

diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 553adfc..b22dfb5 100644 (file)
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -143,6 +143,8 @@ struct GTY(()) ipa_ancestor_jf_data
   int formal_id;
   /* Flag with the same meaning like agg_preserve in ipa_pass_through_data.  */
   unsigned agg_preserved : 1;
+  /* When set, the operation should not have any effect on NULL pointers.  */
+  unsigned keep_null : 1;
 };
 
 /* A jump function for an aggregate part at a given offset, which describes how
@@ -438,6 +440,17 @@ ipa_get_jf_ancestor_type_preserved (struct ipa_jump_func *jfunc)
   return jfunc->value.ancestor.agg_preserved;
 }
 
+/* Return if jfunc represents an operation whether we first check the formal
+   parameter for non-NULLness unless it does not matter because the offset is
+   zero anyway.  */
+
+static inline bool
+ipa_get_jf_ancestor_keep_null (struct ipa_jump_func *jfunc)
+{
+  gcc_checking_assert (jfunc->type == IPA_JF_ANCESTOR);
+  return jfunc->value.ancestor.keep_null;
+}
+
 /* Class for allocating a bundle of various potentially known properties about
    actual arguments of a particular call on stack for the usual case and on
    heap only if there are unusually many arguments.  The data is deallocated

diff --git a/gcc/testsuite/gcc.dg/ipa/pr103083-1.c b/gcc/testsuite/gcc.dg/ipa/pr103083-1.c
new file mode 100644 (file)
index 0000000..e2fbb45
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr103083-1.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -Wno-pointer-to-int-cast" } */
+
+struct b {int b;};
+struct a {int a; struct b b;};
+
+long i;
+
+__attribute__ ((noinline))
+static void test2 (struct b *b)
+{
+  if (((int)b)&4)
+    __builtin_abort ();
+}
+
+__attribute__ ((noinline))
+static void
+test (struct a *a)
+{
+  test2(a? &a->b : 0);
+}
+
+int
+main()
+{
+  test(0);
+  return 0;
+}

diff --git a/gcc/testsuite/gcc.dg/ipa/pr103083-2.c b/gcc/testsuite/gcc.dg/ipa/pr103083-2.c
new file mode 100644 (file)
index 0000000..ae1b905
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr103083-2.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-ipa-bit-cp -fdump-tree-optimized" } */
+
+struct b {int b;};
+struct a {int a; struct b b;};
+
+void remove_any_mention (void);
+
+__attribute__ ((noinline))
+static void test2 (struct b *b)
+{
+  if (b)
+    remove_any_mention ();
+}
+
+__attribute__ ((noinline))
+static void
+test (struct a *a)
+{
+  test2(a? &a->b : 0);
+}
+
+int
+foo()
+{
+  test(0);
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "remove_any_mention" "optimized" } } */