powerpc: Work around gcc miscompilation of __pa() on 64-bit

On 64-bit, __pa(&static_var) gets miscompiled by recent versions of gcc as something like:

        addis 3,2,.LANCHOR1+4611686018427387904@toc@ha
        addi 3,3,.LANCHOR1+4611686018427387904@toc@l

This ends up effectively ignoring the offset, since its bottom 32 bits are zero, and means that the result of __pa() still has 0xC in the top nibble. This happens with gcc 4.8.1, at least.

To work around this, for 64-bit we make __pa() use an AND operator, and for symmetry, we make __va() use an OR operator. Using an AND operator rather than a subtraction ends up with slightly shorter code since it can be done with a single clrldi instruction, whereas it takes three instructions to form the constant (-PAGE_OFFSET) and add it on. (Note that MEMORY_START is always 0 on 64-bit.)

---
 arch/powerpc/Kconfig            |  1 +
 arch/powerpc/include/asm/page.h | 10 ++++++++++
 2 files changed, 11 insertions(+)

index dbd9d3c..9cf59816d 100644
@@ -979,6 +979,7 @@ config RELOCATABLE
 	  must live at a different physical address than the primary
 	  kernel.
 
+# This value must have zeroes in the bottom 60 bits otherwise lots will break
 config PAGE_OFFSET
 	hex
 	default "0xc000000000000000"

index 988c812..b9f4262 100644
@@ -211,9 +211,19 @@ extern long long virt_phys_offset;
 #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
 #define __pa(x) ((unsigned long)(x) - VIRT_PHYS_OFFSET)
 #else
+#ifdef CONFIG_PPC64
+/*
+ * gcc miscompiles (unsigned long)(&static_var) - PAGE_OFFSET
+ * with -mcmodel=medium, so we use & and | instead of - and + on 64-bit.
+ */
+#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET))
+#define __pa(x) ((unsigned long)(x) & 0x0fffffffffffffffUL)
+
+#else /* 32-bit, non book E */
 #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + PAGE_OFFSET - MEMORY_START))
 #define __pa(x) ((unsigned long)(x) - PAGE_OFFSET + MEMORY_START)
 #endif
+#endif
 
 /*
  * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,

1.8.4.rc3

I might argue that this isn't a miscompilation, since -mcmodel=medium assumes everything can be accessed within +/-2G of the toc pointer, but it's definitely a problem since gas and/or ld don't give an overflow error. They would except for the fact that our ABI has a hole in it.

We have relocs that error on 16-bit overflow, eg.
addi 3,2,x@toc
will give an error if x is more than +/-32k from the toc pointer, but @ha and _HA/_HI relocs don't error on 32-bit overflow. (They can't, because they were really designed to be used in HIHGESTA, HIGHERA, HA, LO sequences to build up 64-bit values.)

The proper fix is to define a whole slew of new relocations and reloc specifiers, and modify everything to use them, but that seems like too much bother. I had ideas once upon a time to implement gas and ld options that makes @ha and _HA report overflows, but haven't found one of those round tuits.

Alan Modra
Australia Development Lab, IBM

And there's the bug right there... the assumption that this number that I want to compute is the address of something that can be accessed. It isn't, and gcc shouldn't treat it as such (not unless I do something like subsequently casting it back to a pointer and dereferencing that).

Well, that and the fact that the code in gcc that generates instructions to compute addresses blindly uses addis; addi even when the offset involved is way too large for that to possibly work.

Paul.

No, if you don't have a reloc that can represent this, then the proper fix is to use the existing relocs to load the original symbol address into a register, then *generate* the appropriate 64-bit addition on top of it.

The pointer has been cast to an integer before the addition, it's no longer an object pointer and gcc shouldn't treat it as such, at which point we are in the domain of normal integer arithmetics.

Cheers,
Ben.

I already have a gcc fix to do exactly that. My "proper fix" comment was more to do with the general case. For example, when linking a huge object that overflows _HA relocs right now we silently generate bad code.

Alan Modra
Australia Development Lab, IBM

Ah that is nice indeed :-) In that case I assume we can't have the offset itself be part of the TOC :-) Chicken or egg ?

Not sure what's the right fix here is, we don't want to always reserve enough instructions "space" to do a full 64-bit offset load...

Ben.