On Fri, 18 Nov 2016, Janne Grunau wrote:
On 2016-11-18 09:29:36 +0200, Martin Storsjö wrote:
On Thu, 17 Nov 2016, Janne Grunau wrote:
>On 2016-11-14 00:24:46 +0200, Martin Storsjö wrote:
>>Check these constraints even for non-pic-elf configurations, to
>>catch the issues without having to build/test a pic-elf configuration
>>to find out.
>>---
>> .macro movrelx rd, val, gp
>>+ .ifc \rd,r12
>>+ .ifb \gp
>>+ .error "movrelx rd=r12, needs a manually set gp"
>>+ .endif
>>+ .endif
>>+ .ifc \gp,\rd
>>+ .error "movrelx rd=gp doesn't work"
>>+ .endif
>
>this doesn't work as movrelx is intended, the register holding the GOT
>is saved via the gp register alias. Following should prevent at least
>the using r12 as rd error:
>
>diff --git a/libavutil/arm/asm.S b/libavutil/arm/asm.S
>index 4ac0ea2..49f0a49 100644
>--- a/libavutil/arm/asm.S
>+++ b/libavutil/arm/asm.S
>@@ -185,17 +185,23 @@ T ldr \rd, [\rd]
>
>.macro movrelx rd, val, gp
>#if CONFIG_PIC && defined(__ELF__)
>+ .ifc \rd,r12
>+ .if .Lpic_gp == 12
>+ .error "movrelx rd=r12, needs a manually set gp"
I don't see how this would error out at all, for "movrelx r12, X(foo)"
though, which was the situation I had.
err, yes, I didn't payed attention. Patch below errors out in all
conditions but still can be fooled by using register aliases, using
'ip' instead of r12 and maybe uppercase 'R'.
Janne
---8<---
This catches errors manifesting only on position independent ELF builds
on all configurations. This still can be fooled by using register
aliases or the special name 'ip' for r12.
---
libavutil/arm/asm.S | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/libavutil/arm/asm.S b/libavutil/arm/asm.S
index 4ac0ea2..e2fe463 100644
--- a/libavutil/arm/asm.S
+++ b/libavutil/arm/asm.S
@@ -184,6 +184,25 @@ T ldr \rd, [\rd]
.endm
.macro movrelx rd, val, gp
+ .ifc \rd,\gp
+ .error "movrelx needs two distinct registers"
+ .endif
+ .ifc \rd_\gp,r12_
This needs \() inbetween \rd and _\gp
+ .ifn .Lpic_gp
+ .error "movrelx rd=r12, needs a manually set gp"
+ .elseif .Lpic_gp == 12
+ .error "movrelx rd=r12, needs a manually set gp"
+ .endif
+ .endif
+ .ifnb \gp
+ .ifc \gp,r12
+ .set .Lpic_gp, 12
+ .else
+ .set .Lpic_gp, 1
+ .endif
+ .elseif !.Lpic_gp
+ .set .Lpic_gp, 12
+ .endif
Setting .Lpic_gp here will cause the CONFIG_PIC segment below to try to
.unreq gp. I think it works if you move this block for setting .Lpic_gp to
the end of the macro.
Then we also need to add #if CONFIG_PIC && defined(__ELF__) around the
.unreq in endfunc.
With all those changes in place, I think it works as intended... It's
pretty big and intrusive though.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
