Bug 35705 - [4.3/4.4 Regression] Symbol address check eliminated by C frontend.
Summary: [4.3/4.4 Regression] Symbol address check eliminated by C frontend.
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.3.1
: P3 blocker
Target Milestone: 4.3.1
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2008-03-26 11:58 UTC by Carlos O'Donell
Modified: 2008-04-01 22:17 UTC (History)
7 users (show)

See Also:
Host: hppa-linux
Target: hppa-linux
Build: hppa-linux
Known to work: 4.2.3
Known to fail: 4.3.0
Last reconfirmed: 2008-03-26 12:45:51


Attachments
Regression test for bitwise operations on PLABEL32 address. (1.13 KB, patch)
2008-03-26 13:33 UTC, Carlos O'Donell
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos O'Donell 2008-03-26 11:58:47 UTC
Problem
~~~~~~~~
The C frontend in GCC 4.3.1 appears to assume that the address of a function 
pointer is always aligned, and optimizes away certain bitwise operations. This breaks hppa-linux whose function pointers are either an OPD with a certain bit set, or a plain function pointer. In GCC 4.3 hppa-linux can no longer check to see if a function address is an OPD or not.

Background
~~~~~~~~~~~
The hppa-linux ABI supports non-OPD and OPDs. The difference is that OPDs' have bit 2 set to 1. 

The following testcase shows the change in behaviour between gcc-4_2-branch and gcc-4_3-branch.

cat >> foo.c <<EOF
#include <stdio.h>
#include <stdlib.h>

int main (void) {
 printf ("&printf = %x\n", (unsigned long)&printf);
 if (((unsigned long) &printf) & 3)
   {
     printf ("printf is an OPD (PLABEL32)\n");
   }
 return 0;
}
EOF

carlos@firin:~/fsrc/gcc-work$
/usr/local/tools/bin/hppa-linux-gcc-4.2.4 -o foo foo.c
carlos@firin:~/fsrc/gcc-work$ ./foo
&printf = 119ea
printf is a PLABEL32

carlos@firin:~/fsrc/gcc-work$
/usr/local/tools/bin/hppa-linux-gcc-4.3.1 -o foo foo.c
carlos@firin:~/fsrc/gcc-work$ ./foo
&printf = 119ea

GCC 4.3, even at -O0, reduces "((unsigned long) &printf) & 3)" to "0".

~~~ foo.c.003t.original ~~~
;; Function main (main)
;; enabled by -tree-original

{
 printf ((const char * restrict) (char *) "&printf = %x\n", (long
unsigned int) printf);
 if (0)
   {
     printf ((const char * restrict) (char *) "printf is a PLABEL32\n");
   }
 return 0;
}
~~~
If the if condition is reduced to "0" as early as 003t.original, does
that mean it's the C frontend fault?

This issue is breaks glibc's detection of PLABEL32 symbols during startup
relocation processing. It also likely breaks unwind code in libjava.

Casting a pointer to an integer type is implementation defined, so the above change is certainly a regression.

Why this didn't show up in the testing is another issue. Perhaps we just don't have a test for this (I can correct that pretty easily).

Any ideas how to fix this? Pointers to the change that changed the behaviour?
Comment 1 Richard Biener 2008-03-26 12:45:51 UTC
This is caused by

2007-09-23  Ollie Wild  <aaw@google.com>

        * fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer
        operand.
        (get_pointer_modulus_and_residue): New function.

so I suppose you want to disable this optimization for addresses of functions.
Comment 2 Carlos O'Donell 2008-03-26 13:33:47 UTC
Created attachment 15381 [details]
Regression test for bitwise operations on PLABEL32 address.
Comment 3 Carlos O'Donell 2008-03-26 13:34:20 UTC
Dave,

What do you think of the attached regression test? Is this the right way to test for this behaviour?
Comment 4 Jakub Jelinek 2008-03-26 13:41:16 UTC
If so, then the PA backend is lying with the definition of FUNCTION_BOUNDARY...
Comment 5 Carlos O'Donell 2008-03-26 15:28:23 UTC
Jakub,

Would changing FUNCTION_BOUNDARY to something less than a word alignment have disastrous results e.g. unaligned function start addresses?

Functions should continue to be aligned at word boundaries. The address of a function has nothing to do with the location of the actual function code. In this case the address of the function is an OPD, and such an address requires special bits to be set.

Comment 6 Jim Wilson 2008-03-26 16:37:24 UTC
Subject: Re:  [4.3/4.4 Regression] Symbol address check
 eliminated by C frontend.

jakub at gcc dot gnu dot org wrote:
> ------- Comment #4 from jakub at gcc dot gnu dot org  2008-03-26 13:41 -------
> If so, then the PA backend is lying with the definition of FUNCTION_BOUNDARY...

PA uses function descriptors like Itanium.  A function pointer is 
actually a pointer to a descriptor, and the alignment of the descriptor 
has nothing at all to do with FUNCTION_BOUNDARY.

Also, consider the arm/thumb and mips/mips16 cases, where we distinguish 
between the different types of functions by using the otherwise unused 
low order bits of the address.

Jim
Comment 7 dave 2008-03-26 18:30:26 UTC
Subject: Re:  [4.3/4.4 Regression] Symbol address check eliminated by C frontend.

> If so, then the PA backend is lying with the definition of FUNCTION_BOUNDARY...

Don't see how this matters as function pointers in the hppa runtime don't
generally point at the entry point of the function.  There are function
descriptors and import/export stubs to traverse before you get to the
function itself.

Dave
Comment 8 Andrew Pinski 2008-03-26 18:34:12 UTC
I think the same is true with PPC64-Linux and AIX with function descriptors there too.  Though there is only OPDs function pointers.
Comment 9 dave 2008-03-26 22:55:49 UTC
Subject: Re:  [4.3/4.4 Regression] Symbol address check eliminated by C frontend.

> so I suppose you want to disable this optimization for addresses of functions.

Something like the following?

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 133621)
+++ fold-const.c	(working copy)
@@ -9065,7 +9065,7 @@
 	    }
 	}
 
-      if (DECL_P (expr))
+      if (DECL_P (expr) && TREE_CODE (expr) != FUNCTION_DECL)
 	return DECL_ALIGN_UNIT (expr);
     }
   else if (code == POINTER_PLUS_EXPR)

Dave
Comment 10 rguenther@suse.de 2008-03-26 22:57:15 UTC
Subject: Re:  [4.3/4.4 Regression] Symbol address check
 eliminated by C frontend.

On Wed, 26 Mar 2008, dave at hiauly1 dot hia dot nrc dot ca wrote:

> ------- Comment #9 from dave at hiauly1 dot hia dot nrc dot ca  2008-03-26 22:55 -------
> Subject: Re:  [4.3/4.4 Regression] Symbol address check eliminated by C
> frontend.
> 
> > so I suppose you want to disable this optimization for addresses of functions.
> 
> Something like the following?

Yes.

Richard.

> Index: fold-const.c
> ===================================================================
> --- fold-const.c        (revision 133621)
> +++ fold-const.c        (working copy)
> @@ -9065,7 +9065,7 @@
>             }
>         }
> 
> -      if (DECL_P (expr))
> +      if (DECL_P (expr) && TREE_CODE (expr) != FUNCTION_DECL)
>         return DECL_ALIGN_UNIT (expr);
>      }
>    else if (code == POINTER_PLUS_EXPR)
> 
> Dave
> 
> 
> 

Comment 11 Jakub Jelinek 2008-03-27 09:42:20 UTC
That will unnecessarily punish the majority of targets which use simple
pointers.
I thought you can align functions more than FUNCTION_BOUNDARY by
ASM_OUTPUT_MAX_SKIP_ALIGN, but apparently that's not unconditional.
In that case, we should have a target macro or target hook, which will tell us
alignment of ADDR_EXPR <FUNCTION_DECL>, defaulting to DECL_ALIGN_UNIT of the
FUNCTION_DECL, and get_pointer_modulus_and_residue should use that.
Comment 12 rguenther@suse.de 2008-03-27 09:52:53 UTC
Subject: Re:  [4.3/4.4 Regression] Symbol address check
 eliminated by C frontend.

On Thu, 27 Mar 2008, jakub at gcc dot gnu dot org wrote:

> ------- Comment #11 from jakub at gcc dot gnu dot org  2008-03-27 09:42 -------
> That will unnecessarily punish the majority of targets which use simple
> pointers.
> I thought you can align functions more than FUNCTION_BOUNDARY by
> ASM_OUTPUT_MAX_SKIP_ALIGN, but apparently that's not unconditional.
> In that case, we should have a target macro or target hook, which will tell us
> alignment of ADDR_EXPR <FUNCTION_DECL>, defaulting to DECL_ALIGN_UNIT of the
> FUNCTION_DECL, and get_pointer_modulus_and_residue should use that.

I don't think alignment checks on function pointers are common
(nor portable, a function "pointer" is an opaque object).  So a new
target hook is overkill IMHO.

Richard.
Comment 13 Carlos O'Donell 2008-03-28 00:14:43 UTC
Dave,

I tested the patch you posted in comment #9, and I have no regressions on hppa-linux (C/C++), and it fixes my testcase. I haven't done a full all-languages bootstrap and test.
Comment 14 Carlos O'Donell 2008-03-28 11:39:30 UTC
With the patch in comment #9, a glibc cvs head build completes successfully. The test results show some regressions, but this is almost always the case when switching to a new gcc branch.
Comment 15 John David Anglin 2008-04-01 22:15:26 UTC
Subject: Bug 35705

Author: danglin
Date: Tue Apr  1 22:14:41 2008
New Revision: 133804

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133804
Log:
	PR middle-end/35705
	* fold-const.c (get_pointer_modulus_and_residue): Return modulus 1 if
	the expression is a function address.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c

Comment 16 John David Anglin 2008-04-01 22:17:33 UTC
Subject: Bug 35705

Author: danglin
Date: Tue Apr  1 22:16:49 2008
New Revision: 133805

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133805
Log:
	PR middle-end/35705
	* fold-const.c (get_pointer_modulus_and_residue): Return modulus 1 if
	the expression is a function address.


Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/fold-const.c

Comment 17 John David Anglin 2008-04-01 22:17:57 UTC
Fixed.