diff mbox

[PR,68064] Do not create jump functions with zero alignment

Message ID 20151030124408.GB2460@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor Oct. 30, 2015, 12:44 p.m. UTC
Hi,

in PR 68064, IPA-CP hits an assert upon encountering a jump function
claiming that a pointer has known alignment of zero.  That is actually
what get_pointer_alignment_1 returns when asked what is the alignment
of iftmp.0_1 in:

  <bb 4>:
  # iftmp.0_1 = PHI <0B(2), 2147483648B(3)>
  {anonymous}::fn1 (iftmp.0_1);

I suppose that given the circumstances, it is more-or-less reasonable
answer, even if very weird, so we should check for that possibility.
That is what the patch below does.  Bootstrapped and tested on
x86_64-linux.  OK for trunk?

Thanks,

Martin



2015-10-29  Martin Jambor  <mjambor@suse.cz>

	PR ipa/68064
	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Check that
	alignment iz not zero.

testsuite/
	* g++.dg/torture/pr68064.C: New.

Comments

Richard Biener Oct. 30, 2015, 1 p.m. UTC | #1
On Fri, Oct 30, 2015 at 1:44 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> in PR 68064, IPA-CP hits an assert upon encountering a jump function
> claiming that a pointer has known alignment of zero.  That is actually
> what get_pointer_alignment_1 returns when asked what is the alignment
> of iftmp.0_1 in:
>
>   <bb 4>:
>   # iftmp.0_1 = PHI <0B(2), 2147483648B(3)>
>   {anonymous}::fn1 (iftmp.0_1);
>
> I suppose that given the circumstances, it is more-or-less reasonable
> answer, even if very weird, so we should check for that possibility.
> That is what the patch below does.  Bootstrapped and tested on
> x86_64-linux.  OK for trunk?

Hmm.  So it's overflowing to zero at

          *alignp = ptr_align * BITS_PER_UNIT;

and I'm not sure how many callers are affected.  Having alignment in
bits is of course odd in the first place.

A safe fix would be to set_ptr_info_alignment, capping the alignment
to a value that is safe to scale to bit alignment.  A very forward-looking
fix would be to make alignment be in BITS_PER_UNIT throughout
the whole compiler ...

Richard.

> Thanks,
>
> Martin
>
>
>
> 2015-10-29  Martin Jambor  <mjambor@suse.cz>
>
>         PR ipa/68064
>         * ipa-prop.c (ipa_compute_jump_functions_for_edge): Check that
>         alignment iz not zero.
>
> testsuite/
>         * g++.dg/torture/pr68064.C: New.
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 19846a8..f36e2fd 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1651,6 +1651,7 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
>           unsigned align;
>
>           if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
> +             && align != 0
>               && align % BITS_PER_UNIT == 0
>               && hwi_bitpos % BITS_PER_UNIT == 0)
>             {
> diff --git a/gcc/testsuite/g++.dg/torture/pr68064.C b/gcc/testsuite/g++.dg/torture/pr68064.C
> new file mode 100644
> index 0000000..59b6897
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr68064.C
> @@ -0,0 +1,35 @@
> +// { dg-do compile }
> +
> +template <class Config> class A {
> +public:
> +  class B;
> +  typedef typename Config::template D<A>::type TypeHandle;
> +  static A *Tagged() { return B::New(B::kTagged); }
> +  static TypeHandle Union(TypeHandle);
> +  static TypeHandle Representation(TypeHandle, typename Config::Region *);
> +  bool Is();
> +};
> +
> +template <class Config> class A<Config>::B {
> +  friend A;
> +  enum { kTaggedPointer = 1 << 31, kTagged = kTaggedPointer };
> +  static A *New(int p1) { return Config::from_bitset(p1); }
> +};
> +
> +struct C {
> +  typedef int Region;
> +  template <class> struct D { typedef A<C> *type; };
> +  static A<C> *from_bitset(unsigned);
> +};
> +A<C> *C::from_bitset(unsigned p1) { return reinterpret_cast<A<C> *>(p1); }
> +
> +namespace {
> +int *a;
> +void fn1(A<C> *p1) { A<C>::Union(A<C>::Representation(p1, a)); }
> +}
> +
> +void fn2() {
> +  A<C> b;
> +  A<C> *c = b.Is() ? 0 : A<C>::Tagged();
> +  fn1(c);
> +}
diff mbox

Patch

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 19846a8..f36e2fd 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1651,6 +1651,7 @@  ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	  unsigned align;
 
 	  if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
+	      && align != 0
 	      && align % BITS_PER_UNIT == 0
 	      && hwi_bitpos % BITS_PER_UNIT == 0)
 	    {
diff --git a/gcc/testsuite/g++.dg/torture/pr68064.C b/gcc/testsuite/g++.dg/torture/pr68064.C
new file mode 100644
index 0000000..59b6897
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr68064.C
@@ -0,0 +1,35 @@ 
+// { dg-do compile }
+
+template <class Config> class A {
+public:
+  class B;
+  typedef typename Config::template D<A>::type TypeHandle;
+  static A *Tagged() { return B::New(B::kTagged); }
+  static TypeHandle Union(TypeHandle);
+  static TypeHandle Representation(TypeHandle, typename Config::Region *);
+  bool Is();
+};
+
+template <class Config> class A<Config>::B {
+  friend A;
+  enum { kTaggedPointer = 1 << 31, kTagged = kTaggedPointer };
+  static A *New(int p1) { return Config::from_bitset(p1); }
+};
+
+struct C {
+  typedef int Region;
+  template <class> struct D { typedef A<C> *type; };
+  static A<C> *from_bitset(unsigned);
+};
+A<C> *C::from_bitset(unsigned p1) { return reinterpret_cast<A<C> *>(p1); }
+
+namespace {
+int *a;
+void fn1(A<C> *p1) { A<C>::Union(A<C>::Representation(p1, a)); }
+}
+
+void fn2() {
+  A<C> b;
+  A<C> *c = b.Is() ? 0 : A<C>::Tagged();
+  fn1(c);
+}