Bug 49169 - ARM: optimisations strip the Thumb/ARM mode bit off function pointers
Summary: ARM: optimisations strip the Thumb/ARM mode bit off function pointers
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2011-05-26 02:13 UTC by Michael Hope
Modified: 2012-07-31 01:00 UTC (History)
4 users (show)

See Also:
Host:
Target: arm
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-07-08 15:04:22


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hope 2011-05-26 02:13:27 UTC
ARM devices encode the instruction set mode in the LSB of the function address.  Functions are word aligned on ARM.  If you try to test the LSB of a function pointer then GCC assumes that the two least significant bits are zero and optimises away the test.

This problem is seen in Mono and was originally reported at:
 https://bugs.launchpad.net/ubuntu/+source/gcc-4.5/+bug/721531

A reduced test case is:

void main() {
        void *p = main;
        if ((int)p & 1) printf ("HIT!\n");
}

When compiled with -march=armv7-a -mthumb -O0 then the word 'HIT!' will show.  When compiled with -O2, the branch is not taken.

The problem does not occur in 4.4.5.  It does occur in 4.5.2, 4.6.0, and trunk r174044.
Comment 1 Mikael Pettersson 2011-05-26 07:56:05 UTC
Try passing the function pointer through an opaque identity transform:

    asm("" : "=r"(p) : "0"(main));
    if ((uintptr_t)p & 1) /* do thumb case */
Comment 2 Richard Biener 2011-05-26 08:20:22 UTC
There are several places in the compiler where we assume DECL_ALIGN
constraints the lower bits of the address of the DECL.

See several similar bugs in the past (PR47239 comes to my mind).

fold-const.c:get_pointer_modulus_and_residue looks suspicious to me here,
so you might want to try

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 174266)
+++ gcc/fold-const.c    (working copy)
@@ -9219,7 +9219,8 @@ get_pointer_modulus_and_residue (tree ex
   *residue = 0;
 
   code = TREE_CODE (expr);
-  if (code == ADDR_EXPR)
+  if (code == ADDR_EXPR
+      && TREE_CODE (TREE_OPERAND (expr, 0)) != FUNCTION_DECL)
     {
       unsigned int bitalign;
       bitalign = get_object_alignment_1 (TREE_OPERAND (expr, 0), residue);
Comment 3 Richard Biener 2011-05-26 08:30:21 UTC
Btw, we finally should introduce a target hook for this I think.
Comment 4 Richard Sandiford 2011-06-07 07:32:37 UTC
(In reply to comment #3)
> Btw, we finally should introduce a target hook for this I think.

Thanks for the patch in comment #2.  How strongly do you feel
about the hook though?  In PR35705, it sounded like a lot of
targets actually need an opt-out for functions, either because
of ISA encoding (ARM, MIPS, SH) or because of function
descriptors (IA-64, PA, PPC).

I notice that ARM and mcore also have optimisation-dependent
FUNCTION_BOUNDARYs.  Arguably (very arguably) that's a bug,
and they should be using align_functions instead.  But if we
make a deliberate decision to honour DECL_ALIGN on functions,
then FUNCTION_BOUNDARY really will be an ABI property.
I'm just worried that the combination of that and the need
to identify exactly which targets should define the hook
might be more hassle than the optimisation is worth.

You said in that bug that masking function addresses isn't
likely to be a common operation, and TBH, I still agree.

Richard
Comment 5 Richard Sandiford 2011-06-27 09:33:10 UTC
Author: rsandifo
Date: Mon Jun 27 09:33:06 2011
New Revision: 175427

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175427
Log:
gcc/
2011-07-24  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49169
	* fold-const.c (get_pointer_modulus_and_residue): Don't rely on
	the alignment of function decls.

gcc/testsuite/
2011-07-24  Michael Hope  <michael.hope@linaro.org>
	    Richard Sandiford  <richard.sandiford@linaro.org>

	PR tree-optimization/49169
	* gcc.dg/torture/pr49169.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr49169.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 jye2 2011-09-19 08:13:09 UTC
Author: jye2
Date: Mon Sep 19 08:13:02 2011
New Revision: 178955

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178955
Log:
2011-09-19  Jiangning Liu  <jiangning.liu@arm.com>

	Backport r175427 from mainline
	2011-06-27  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49169
	* fold-const.c (get_pointer_modulus_and_residue): Don't rely on
	the alignment of function decls.

2011-09-19  Jiangning Liu  <jiangning.liu@arm.com>

	Backport r175208 from mainline
	2011-06-20  Ramana Radhakrishnan  <ramana.radhakrishnan@linaro.org>

	PR target/49385
	* config/arm/thumb2.md (*thumb2_movhi_insn): Make sure atleast
	one of the operands is a register.

2011-09-19  Jiangning Liu  <jiangning.liu@arm.com>

	Backport r174803 from mainline
	2011-06-08  Julian Brown  <julian@codesourcery.com>

	* config/arm/arm.c (arm_libcall_uses_aapcs_base): Use correct ABI
	for double-precision helper functions in hard-float mode if only
	single-precision arithmetic is supported in hardware.


Modified:
    branches/ARM/embedded-4_6-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-4_6-branch/gcc/config/arm/arm.c
    branches/ARM/embedded-4_6-branch/gcc/config/arm/thumb2.md
    branches/ARM/embedded-4_6-branch/gcc/fold-const.c
Comment 7 Ramana Radhakrishnan 2012-07-31 01:00:46 UTC
This should be marked FIXED as of 4.7.0 . 

Ramana