* [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
@ 2013-08-27 6:07 Paul Mackerras
2013-08-27 7:12 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Paul Mackerras @ 2013-08-27 6:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alan Modra
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.)
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/page.h | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dbd9d3c..9cf59816d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -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"
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 988c812..b9f4262 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
2013-08-27 6:07 [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit Paul Mackerras
@ 2013-08-27 7:12 ` Alan Modra
2013-08-27 9:03 ` Paul Mackerras
2013-09-01 23:59 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 6+ messages in thread
From: Alan Modra @ 2013-08-27 7:12 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
On Tue, Aug 27, 2013 at 04:07:49PM +1000, Paul Mackerras wrote:
> 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
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
2013-08-27 7:12 ` Alan Modra
@ 2013-08-27 9:03 ` Paul Mackerras
2013-09-01 23:59 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 6+ messages in thread
From: Paul Mackerras @ 2013-08-27 9:03 UTC (permalink / raw)
To: Alan Modra; +Cc: linuxppc-dev
On Tue, Aug 27, 2013 at 04:42:35PM +0930, Alan Modra wrote:
> On Tue, Aug 27, 2013 at 04:07:49PM +1000, Paul Mackerras wrote:
> > 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
>
> I might argue that this isn't a miscompilation, since -mcmodel=medium
> assumes everything can be accessed within +/-2G of the toc pointer,
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).
> 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.
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
2013-08-27 7:12 ` Alan Modra
2013-08-27 9:03 ` Paul Mackerras
@ 2013-09-01 23:59 ` Benjamin Herrenschmidt
2013-09-02 2:31 ` Alan Modra
1 sibling, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2013-09-01 23:59 UTC (permalink / raw)
To: Alan Modra; +Cc: linuxppc-dev, Paul Mackerras
On Tue, 2013-08-27 at 16:42 +0930, Alan Modra wrote:
> 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.
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
2013-09-01 23:59 ` Benjamin Herrenschmidt
@ 2013-09-02 2:31 ` Alan Modra
2013-09-02 5:06 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2013-09-02 2:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
On Mon, Sep 02, 2013 at 09:59:12AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2013-08-27 at 16:42 +0930, Alan Modra wrote:
> > 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.
>
> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit
2013-09-02 2:31 ` Alan Modra
@ 2013-09-02 5:06 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2013-09-02 5:06 UTC (permalink / raw)
To: Alan Modra; +Cc: linuxppc-dev, Paul Mackerras
On Mon, 2013-09-02 at 12:01 +0930, Alan Modra wrote:
> > 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.
>
> 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.
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-02 5:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-27 6:07 [PATCH] powerpc: Work around gcc miscompilation of __pa() on 64-bit Paul Mackerras
2013-08-27 7:12 ` Alan Modra
2013-08-27 9:03 ` Paul Mackerras
2013-09-01 23:59 ` Benjamin Herrenschmidt
2013-09-02 2:31 ` Alan Modra
2013-09-02 5:06 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).