Bug 65956 - [5/6 Regression] Another ARM overaligned arg passing issue
Summary: [5/6 Regression] Another ARM overaligned arg passing issue
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 5.1.0
: P1 normal
Target Milestone: 5.2
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-30 21:17 UTC by Jakub Jelinek
Modified: 2016-01-21 08:19 UTC (History)
1 user (show)

See Also:
Host:
Target: arm-linux-gnueabi, arm-eabi
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-05-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2015-04-30 21:17:20 UTC
The following testcase is miscompiled at -O2 on armhfp, starting with r221348, and in this case r221795 didn't fix it.  Just hope it is the same thing that breaks armhfp profiledbootstrap.  The difference on the testcase is just:
 	mov	r0, r5
-	ldr	r1, [sp, #24]
+	ldr	r2, [sp, #24]
 	bl	fn1
which means that we pass the second argument to fn1 in a wrong register, supposedly because the type is void * aligned to long long alignment.

struct A { char *a; int b; long long c; };
char v[3];

__attribute__((noinline, noclone)) void
fn1 (char *x, char *y)
{
  if (x != &v[1] || y != &v[2])
    __builtin_abort ();
  v[1]++;
}

__attribute__((noinline, noclone)) int
fn2 (char *x)
{
  asm volatile ("" : "+g" (x) : : "memory");
  return x == &v[0];
}

__attribute__((noinline, noclone)) void
fn3 (const char *x)
{
  if (x[0] != 0)
    __builtin_abort ();
}

static struct A
foo (const char *x, struct A y, struct A z)
{
  struct A r = { 0, 0, 0 };
  if (y.b && z.b)
    {
      if (fn2 (y.a) && fn2 (z.a))
	switch (x[0])
	  {
	  case '|':
	    break;
	  default:
	    fn3 (x);
	  }
      fn1 (y.a, z.a);
    }
  return r;
}

__attribute__((noinline, noclone)) int
bar (int x, struct A *y)
{
  switch (x)
    {
    case 219:
      foo ("+", y[-2], y[0]);
    case 220:
      foo ("-", y[-2], y[0]);
    }
}

int
main ()
{
  struct A a[3] = { { &v[1], 1, 1LL }, { &v[0], 0, 0LL }, { &v[2], 2, 2LL } };
  bar (220, a + 2);
  if (v[1] != 1)
    __builtin_abort ();
  return 0;
}
Comment 1 Jakub Jelinek 2015-04-30 23:12:16 UTC
Untested fix:

--- gcc/tree-sra.c.jj	2015-04-20 14:35:47.000000000 +0200
+++ gcc/tree-sra.c	2015-05-01 01:08:34.092636496 +0200
@@ -4427,7 +4427,11 @@ turn_representatives_into_adjustments (v
 	      gcc_assert (repr->base == parm);
 	      adj.base_index = index;
 	      adj.base = repr->base;
-	      adj.type = repr->type;
+	      /* Drop any special alignment on the type if it's not on the
+		 main variant.  This avoids issues with weirdo ABIs like
+		 AAPCS.  */
+	      adj.type = build_qualified_type (TYPE_MAIN_VARIANT (repr->type),
+					       TYPE_QUALS (repr->type));
 	      adj.alias_ptr_type = reference_alias_ptr_type (repr->expr);
 	      adj.offset = repr->offset;
 	      adj.by_ref = (POINTER_TYPE_P (TREE_TYPE (repr->base))

Though, wonder how many workaround we'll need for this AAPCS bogosity :(.
Comment 2 Jakub Jelinek 2015-05-11 08:16:11 UTC
A patch for the ARM backend instead has been posted in
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00278.html
Comment 3 Alan Lawrence 2015-07-06 16:58:47 UTC
Author: alalaw01
Date: Mon Jul  6 16:58:16 2015
New Revision: 225465

URL: https://gcc.gnu.org/viewcvs?rev=225465&root=gcc&view=rev
Log:
[ARM] PR/65956 AAPCS update for alignment attribute

gcc/:
	PR target/65956
	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
	alignment attribute, exploring one level down for records and arrays.

gcc/testsuite/:

	* gcc.target/arm/aapcs/align1.c: New.
	* gcc.target/arm/aapcs/align_rec1.c: New.
	* gcc.target/arm/aapcs/align2.c: New.
	* gcc.target/arm/aapcs/align_rec2.c: New.
	* gcc.target/arm/aapcs/align3.c: New.
	* gcc.target/arm/aapcs/align_rec3.c: New.
	* gcc.target/arm/aapcs/align4.c: New.
	* gcc.target/arm/aapcs/align_rec4.c: New.
	* gcc.target/arm/aapcs/align_vararg1.c: New.
	* gcc.target/arm/aapcs/align_vararg2.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align1.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align2.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align3.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align4.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg1.c
    trunk/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog
Comment 4 Alan Lawrence 2015-07-06 17:06:31 UTC
Author: alalaw01
Date: Mon Jul  6 17:06:00 2015
New Revision: 225466

URL: https://gcc.gnu.org/viewcvs?rev=225466&root=gcc&view=rev
Log:
Fix eipa_src AAPCS issue (PR target/65956)

2015-05-05  Jakub Jelinek  <jakub@redhat.com>

	PR target/65956
	* gcc.c-torture/execute/pr65956.c: New test.


Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr65956.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 5 Alan Lawrence 2015-07-06 17:32:38 UTC
Author: alalaw01
Date: Mon Jul  6 17:32:07 2015
New Revision: 225469

URL: https://gcc.gnu.org/viewcvs?rev=225469&root=gcc&view=rev
Log:
2015-07-06  Alan Lawrence  <alan.lawrence@arm.com>

	Backport from mainline r225465
	2015-07-06  Alan Lawrence  <alan.lawrence@arm.com>

gcc/:

	PR target/65956
	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
	alignment attribute, exploring one level down for records and arrays.

gcc/testsuite/:

	* gcc.target/arm/aapcs/align1.c: New.
	* gcc.target/arm/aapcs/align_rec1.c: New.
	* gcc.target/arm/aapcs/align2.c: New.
	* gcc.target/arm/aapcs/align_rec2.c: New.
	* gcc.target/arm/aapcs/align3.c: New.
	* gcc.target/arm/aapcs/align_rec3.c: New.
	* gcc.target/arm/aapcs/align4.c: New.
	* gcc.target/arm/aapcs/align_rec4.c: New.
	* gcc.target/arm/aapcs/align_vararg1.c: New.
	* gcc.target/arm/aapcs/align_vararg2.c: New.


Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align1.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align2.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align3.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align4.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg1.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg2.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/arm/arm.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 6 Alan Lawrence 2015-07-06 17:38:22 UTC
Author: alalaw01
Date: Mon Jul  6 17:37:50 2015
New Revision: 225470

URL: https://gcc.gnu.org/viewcvs?rev=225470&root=gcc&view=rev
Log:
Backport r225466: tests from 'Fix eipa_src AAPCS issue (PR target/65956)'

2015-05-05  Jakub Jelinek  <jakub@redhat.com>
	
	PR target/65956
	* gcc.c-torture/execute/pr65956.c: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.c-torture/execute/pr65956.c
Modified:
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 7 Ramana Radhakrishnan 2015-07-15 08:01:21 UTC
Fixed for 5.2 ?