[FFmpeg-devel] [PATCH] avcodec/aarch64: Access externs via GOT with PIC

[Martin Storsjö]

Hi,

Thanks for your patch! While the patch probably is worthwhile to pursue, 
ffmpeg does work on Android 6 and newer (with the aarch64 assembly), so 
there's some gaps/misses in the reasoning in the patch description.

On Sun, 10 Jul 2022, Triang3l wrote:

> To support PIC, all AArch64 assembly code in FFmpeg uses the `movrel` 
> macro to obtain addresses of labels, such as lookup table constants, in 
> a way that with CONFIG_PIC being 1, PC-relative addresses of labels are 
> computed via the `adrp` instruction.

> This approach, however, is suitable only for labels defined in the same 
> object file. For `adrp` to work directly between object files, the 
> linker has to perform a text relocation.

No, it doesn't. It's fully possible to build such libraries without text 
relocations currently.

My memory of the situation was that we're linking our shared libraries 
with -Wl,-Bsymbolic, which means that those symbol references are bound at 
link time, so the offset from adrp to the target symbol is fixed at link 
time, and no text relocation is needed.

However I did try to link such shared libraries, removing the 
-Wl,-Bsymbolic argument, and it still succeeds. So I'm a bit unsure what 
really makes it work for me when it isn't working for you.

With Android NDK r24, I configured a build with "-target-os=android 
--arch=aarch64 --cc=aarch64-linux-android32-clang --enable-cross-compile 
--enable-shared", and successfully build that. The built libavcodec.so.59 
does not have text relocations. Can you reproduce and confirm this bit?

> And in my scenario (libavcodec being a static library linked 
> to a shared library, though I'm not sure if this is relevant),

I think this might be quite relevant. Does adding -Wl,-Bsymbolic to the 
linker invocation when linking in the static libavcodec into your shared 
library fix the issue?

(I'm not necessarily arguing that we shouldn't fix this issue - but I want 
to know exactly what differs in your setup and what detail exactly makes 
it work in our current default builds which is missing in yours.)

> Windows has no concept of PIC, and Windows builds should be done 
> with CONFIG_PIC being 0, so it doesn't need to be handled.

While Windows doesn't really have proper PIC, common builds of ffmpeg on 
Windows on aarch64 do end up with CONFIG_PIC set to 1, so please do handle 
that in the movrelx macro too, expanding to the same as the current movrel 
macro for those cases.

// Martin