diff mbox series

ipa-cp: Punt for too large offsets [PR108605]

Message ID Y+dlcOSzUo6Jwwyg@tucnak
State New
Headers show
Series ipa-cp: Punt for too large offsets [PR108605] | expand

Commit Message

Jakub Jelinek Feb. 11, 2023, 9:52 a.m. UTC 
Hi!

Seems most of IPA uses unsigned type for byte offsets
ipa-param-manipulation.h:  unsigned unit_offset;
ipa-param-manipulation.h:  unsigned unit_offset;
ipa-param-manipulation.h:  void register_replacement (tree base, unsigned unit_offset, tree replacement);
ipa-param-manipulation.h:  tree lookup_replacement (tree base, unsigned unit_offset);
ipa-param-manipulation.h:						    unsigned unit_offset);
ipa-prop.h:  unsigned unit_offset;
ipa-prop.h:  tree get_value (int index, unsigned unit_offset, bool by_ref) const;
ipa-prop.h:  tree get_value (int index, unsigned unit_offset) const;
ipa-prop.h:  const ipa_argagg_value *get_elt (int index, unsigned unit_offset) const;
ipa-cp.cc:ipa_argagg_value_list::get_elt (int index, unsigned unit_offset) const
ipa-cp.cc:  unsigned prev_unit_offset = 0;
ipa-cp.cc:ipa_argagg_value_list::get_value (int index, unsigned unit_offset) const
ipa-cp.cc:ipa_argagg_value_list::get_value (int index, unsigned unit_offset,
ipa-cp.cc:      unsigned other_offset = other.m_elts[i].unit_offset;
ipa-cp.cc:  unsigned prev_unit_offset = 0;
ipa-cp.cc:  unsigned prev_unit_offset = 0;
ipa-cp.cc:      unsigned this_offset = elts[i].unit_offset;
ipa-cp.cc:  unsigned prev_unit_offset = 0;
ipa-cp.cc:	  unsigned unit_offset = aglat->offset / BITS_PER_UNIT;
ipa-cp.cc:  unsigned prev_unit_offset = 0;
ipa-param-manipulation.cc:  unsigned unit_offset;
ipa-param-manipulation.cc:isra_get_ref_base_and_offset (tree expr, tree *base_p, unsigned *unit_offset_p)
ipa-param-manipulation.cc:						  unsigned unit_offset,
ipa-param-manipulation.cc:						  unsigned unit_offset)
ipa-param-manipulation.cc:ipa_param_body_adjustments::lookup_replacement (tree base, unsigned unit_offset)
ipa-param-manipulation.cc:  unsigned unit_offset;
ipa-prop.cc:      unsigned unit_offset = bit_offset / BITS_PER_UNIT;
ipa-sra.cc:  unsigned unit_offset;
ipa-sra.cc:  unsigned unit_offset;
ipa-sra.cc:			     unsigned unit_offset, unsigned unit_size)
ipa-sra.cc:      unsigned offset = argacc->unit_offset + delta_offset;
so before converting a HOST_WIDE_INT bit offset to unsigned byte offset
we need to punt for too large offsets.  Some places do that, e.g.
isra_get_ref_base_and_offset has
  if (offset < 0 || (offset / BITS_PER_UNIT) > UINT_MAX)
    return false;
but ipa_agg_value_from_jfunc doesn't.

The following patch fixes that, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2023-02-11  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/108605
	* ipa-cp.cc (ipa_agg_value_from_jfunc): Return NULL_TREE also if
	item->offset bit position is too large to be representable as
	unsigned int byte position.

	* c-c++-common/pr108605.c: New test.


	Jakub

Comments

Richard Biener Feb. 11, 2023, 2:10 p.m. UTC | #1 
> Am 11.02.2023 um 10:53 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> Seems most of IPA uses unsigned type for byte offsets
> ipa-param-manipulation.h:  unsigned unit_offset;
> ipa-param-manipulation.h:  unsigned unit_offset;
> ipa-param-manipulation.h:  void register_replacement (tree base, unsigned unit_offset, tree replacement);
> ipa-param-manipulation.h:  tree lookup_replacement (tree base, unsigned unit_offset);
> ipa-param-manipulation.h:                            unsigned unit_offset);
> ipa-prop.h:  unsigned unit_offset;
> ipa-prop.h:  tree get_value (int index, unsigned unit_offset, bool by_ref) const;
> ipa-prop.h:  tree get_value (int index, unsigned unit_offset) const;
> ipa-prop.h:  const ipa_argagg_value *get_elt (int index, unsigned unit_offset) const;
> ipa-cp.cc:ipa_argagg_value_list::get_elt (int index, unsigned unit_offset) const
> ipa-cp.cc:  unsigned prev_unit_offset = 0;
> ipa-cp.cc:ipa_argagg_value_list::get_value (int index, unsigned unit_offset) const
> ipa-cp.cc:ipa_argagg_value_list::get_value (int index, unsigned unit_offset,
> ipa-cp.cc:      unsigned other_offset = other.m_elts[i].unit_offset;
> ipa-cp.cc:  unsigned prev_unit_offset = 0;
> ipa-cp.cc:  unsigned prev_unit_offset = 0;
> ipa-cp.cc:      unsigned this_offset = elts[i].unit_offset;
> ipa-cp.cc:  unsigned prev_unit_offset = 0;
> ipa-cp.cc:      unsigned unit_offset = aglat->offset / BITS_PER_UNIT;
> ipa-cp.cc:  unsigned prev_unit_offset = 0;
> ipa-param-manipulation.cc:  unsigned unit_offset;
> ipa-param-manipulation.cc:isra_get_ref_base_and_offset (tree expr, tree *base_p, unsigned *unit_offset_p)
> ipa-param-manipulation.cc:                          unsigned unit_offset,
> ipa-param-manipulation.cc:                          unsigned unit_offset)
> ipa-param-manipulation.cc:ipa_param_body_adjustments::lookup_replacement (tree base, unsigned unit_offset)
> ipa-param-manipulation.cc:  unsigned unit_offset;
> ipa-prop.cc:      unsigned unit_offset = bit_offset / BITS_PER_UNIT;
> ipa-sra.cc:  unsigned unit_offset;
> ipa-sra.cc:  unsigned unit_offset;
> ipa-sra.cc:                 unsigned unit_offset, unsigned unit_size)
> ipa-sra.cc:      unsigned offset = argacc->unit_offset + delta_offset;
> so before converting a HOST_WIDE_INT bit offset to unsigned byte offset
> we need to punt for too large offsets.  Some places do that, e.g.
> isra_get_ref_base_and_offset has
>  if (offset < 0 || (offset / BITS_PER_UNIT) > UINT_MAX)
>    return false;
> but ipa_agg_value_from_jfunc doesn't.
> 
> The following patch fixes that, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?

Ok.

Richard 

> 2023-02-11  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR ipa/108605
>    * ipa-cp.cc (ipa_agg_value_from_jfunc): Return NULL_TREE also if
>    item->offset bit position is too large to be representable as
>    unsigned int byte position.
> 
>    * c-c++-common/pr108605.c: New test.
> 
> --- gcc/ipa-cp.cc.jj    2023-01-02 09:32:34.699107365 +0100
> +++ gcc/ipa-cp.cc    2023-02-10 19:10:21.565419411 +0100
> @@ -1982,7 +1982,9 @@ ipa_agg_value_from_jfunc (ipa_node_param
>   tree value = NULL_TREE;
>   int src_idx;
> 
> -  if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
> +  if (item->offset < 0
> +      || item->jftype == IPA_JF_UNKNOWN
> +      || item->offset >= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT)
>     return NULL_TREE;
> 
>   if (item->jftype == IPA_JF_CONST)
> --- gcc/testsuite/c-c++-common/pr108605.c.jj    2023-02-10 19:23:01.449592103 +0100
> +++ gcc/testsuite/c-c++-common/pr108605.c    2023-02-10 19:22:43.773845986 +0100
> @@ -0,0 +1,24 @@
> +/* PR ipa/108605 */
> +/* { dg-do compile { target { lp64 || llp64 } } } */
> +/* { dg-options "-O2" } */
> +
> +struct S {
> +  char a, b, c;
> +  int d[__INT_MAX__], e;
> +};
> +
> +void
> +foo (struct S *s)
> +{
> +  if (s->b && s->c != 0)
> +    __builtin_abort ();
> +}
> +
> +void
> +bar (void)
> +{
> +  struct S s[2];
> +  s[0].a = 0;
> +  s[0].e = 0;
> +  foo (s);
> +}
> 
>    Jakub
>
diff mbox series

Patch

--- gcc/ipa-cp.cc.jj	2023-01-02 09:32:34.699107365 +0100
+++ gcc/ipa-cp.cc	2023-02-10 19:10:21.565419411 +0100
@@ -1982,7 +1982,9 @@  ipa_agg_value_from_jfunc (ipa_node_param
   tree value = NULL_TREE;
   int src_idx;
 
-  if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
+  if (item->offset < 0
+      || item->jftype == IPA_JF_UNKNOWN
+      || item->offset >= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT)
     return NULL_TREE;
 
   if (item->jftype == IPA_JF_CONST)
--- gcc/testsuite/c-c++-common/pr108605.c.jj	2023-02-10 19:23:01.449592103 +0100
+++ gcc/testsuite/c-c++-common/pr108605.c	2023-02-10 19:22:43.773845986 +0100
@@ -0,0 +1,24 @@ 
+/* PR ipa/108605 */
+/* { dg-do compile { target { lp64 || llp64 } } } */
+/* { dg-options "-O2" } */
+
+struct S {
+  char a, b, c;
+  int d[__INT_MAX__], e;
+};
+
+void
+foo (struct S *s)
+{
+  if (s->b && s->c != 0)
+    __builtin_abort ();
+}
+
+void
+bar (void)
+{
+  struct S s[2];
+  s[0].a = 0;
+  s[0].e = 0;
+  foo (s);
+}