Mips - Mark the section .eh_frame as writeable for pic

[dean]

Summary

This patch aims to solve the issue of loading libraries having an .eh_frame with relocations in android mips/mips64 platforms. The problem has been originally seen in the vanilla aosp mips64 emulator, when emitting objects with the section .eh_frame, then the mclinker shipped within the AOSP issues the warning `creating a DT_TEXTREL in a shared object'' when creating a shared library out of the object file, while the dynamic linker fails to load the created library issuing the error: `dlopen failed: text relocations (DT_TEXTREL) found in 64-bit ELF file''. The warning by the linker is issued for both mips32 & mips64, but for mips32 the library is however successfully loaded by the emulator.
Noting that the same problem does not show up when utilising the gnu linker, a bug has been reported to the mclinker community [1]. It turned out that a conceptually similar patch has been merged for the LLVM fork part of the Android NDK [2] and this problem does not occur when using the NDK toolchain, but only with the LLVM libraries inside Android. It has also been pointed out that the GNU linker is able to transform the absolute addresses in the .eh_frame in relative ones, while this functionality is not currently provided by the mclinker [3].
As a way to address the issue, this patch marks the section .eh_frame as writeable in ELF objects for mips/mips64, so that required relocations can be applied by the dynamic linker at runtime. This resembles the behaviour in the patch in the Android NDK, except that in the NDK is the section for the exception handlers the one marked, while in this case the problem is with .eh_frame. I do not know the rationale for this difference.

References:
[1] Bug report for the MClinker: https://groups.google.com/forum/#!topic/mclinker/9eJJGDjIwy4
[2] Andrew Hsieh patch for the NDK: https://android.googlesource.com/toolchain/llvm/+/48604b20a5daaa114c9cad67f1811c321ec0d947%5E!/
[3] Conversion of absolute FDE relocations into relative ones, MClinker mailing list - https://groups.google.com/forum/#!topic/mclinker/mpqg4lFjlfE

[dean]

Source indentation

[grosbach]

So this is a compiler workaround for an MCLinker bug/restriction/missing feature? Why do we want to do that?

[danalbert]

By "GNU linker" do you mean gold, bfd, or both? Both are a part of gnu binutils.

From what you wrote, it sounds like this is only a problem for mclinker. If you're only looking to fix Android (or perhaps you were just using that as an example) then this patch is unnecessary because we don't use mclinker. Note that the NDK fork of LLVM has been undone; new NDKs will ship the same LLVM as the platform (which is a pure upstream LLVM + a few renderscript patches).

[dean]

yes, I also see it as a workaround until the problem will not be properly addressed. I do not get whether this must be a responsibility of the linker, I was expecting with PIC there should be no absolute addresses in the code in the first place, can you confirm this is not the case? Even if it was, this should be resolved somewhere else in the compiler, my aim is to have a way to proceed until is not.

[dean]

@danalbert: thanks for the clarification. I've tested this with the bfd. In Android, the mclinker is currently invoked by the renderscript runtime, will not this be the case in the future?

[grosbach]

If you're arguing that the linker should not have to do this, then please make a case for that. You may well be right (I don't know ELF well enough to have an opinion on that).

The original description is that this is working around a problem only present in the MCLinker, which is not a part of the LLVM project, nor is it the system linker on the platform referenced. As such, workarounds for it aren't really very compelling.

[dsanders]

Hi,

It was Android that asked us to get read-only .eh_frame working properly for Mips so I might have two opposing requests from the same project.

Can we solve this by making the encodings relative for MCLinker or fixing MCLinker?

[danalbert]

In D13104#251952, @dean wrote:

@danalbert: In Android, the mclinker is currently invoked by the renderscript runtime, will not this be the case in the future?

Ah, you're correct. My mistake.

[• rafael]

+ The section .eh_frame exhibits absolute addresses which require fixups.
+ While the gnu linker can transform them in relative references at
+ linking time, at the moment the mclinker does not provide this
+ functionality. Setting the section as writable allows the fixups to be
+ // resolved at run time.
+ if(RelocM == Reloc::PIC_){
+ eh_frame_writable = true;
+ }

Can't we change our created .eh_frame to use relative references?

Failing that, we this should be more specific (android only at least).

Cheers,
Rafael

[logan]

In D13104#251958, @dsanders wrote:

It was Android that asked us to get read-only .eh_frame working properly for Mips so I might have two opposing requests from the same project.

Hi @dsanders,

I guess that you got this impression due to rL209907. I remembered that when I was writing that patch, some (old) versions of Android libc can't relocate the read only sections properly[1]. As a result, we have use DW_EH_PE_indirect to avoid dynamic relocations in .eh_frame sections (like what gcc did.) I was only testing clang+ld.bfd and clang+ld.gold at the time.

If I read this change correctly, it seems that it is trying to workaround another issue. For unknown reasons, the compiler is generating some ABS relocations. ld.bfd and ld.gold will statically relocate these symbols. However, ld.mcld will leave them unchanged and result in the warnings or dlopen() failures on MIPS64. This change goes a step further simply by marking this section as writable. I think this will work but suboptimal.

Logan

[1] IIRC, it was Android 3.1 or 4.0. BTW, the newer Android libc (>4.1?) fixed the issue so that some relocation can be applied to read only sections; however, it is discouraged to have dynamic relocations on read only sections because higher memory usage.

[dsanders]

@logan: I also had a request directly from Android (via our MIPS Android team).

http://reviews.llvm.org/rL238863 is the commit that made TTypeEncoding indirect and pc-relative. While developing that patch, I tried making FDECFIEncoding pc-relative and had to revert that part. IIRC it was because ld.bfd wouldn't accept it since it was expecting to rewrite absolute references into pc relative references.

[joerg]

MIPS doesn't have relative relocations that can cross sections. This is the reason why the normal relative encoding is not usable.

[logan]

In D13104#252365, @logan wrote:

I was only testing clang+ld.bfd and clang+ld.gold at the time.

Correction: I was wrong in the previous comment. I only tested clang+ld.bfd at the time.

I tried to use ld.gold to link the attached relocatable file these days [1]. However there are two problems:

• It seems that ld.gold won't be built for mipsel-linux-gnueabi by default (with --enable-gold.)

• Even if I built ld.gold with some hacks, it will raise an internal error while reading the attached relocatable file [1].

So, it seems that the BFD linker is the only working linker.

BTW, IMHO, it worth to create a bug report on the MIPS PIC relocations at the Bugzilla. It will make it easier to search for the discussion on this topic.

[1] Bug report for the MClinker: https://groups.google.com/forum/#!topic/mclinker/9eJJGDjIwy4

[emaste]

We also face this problem on FreeBSD/mips64 when trying to link with lld.

[joerg]

BTW, I recently spend some time slapping GNU ld in NetBSD into shape so that it can properly support read-only .eh_frame even on MIPS. You might want to look at adopting similar changes.

lib/MC/MCObjectFileInfo.cpp

 }
 
 void MCObjectFileInfo::initELFMCObjectFileInfo(Triple T) {
+  bool eh_frame_writable = false;
+
   switch (T.getArch()) {
   case Triple::mips:
   case Triple::mipsel:

                     dwarf::DW_EH_PE_sdata4;
     // We don't support PC-relative LSDA references in GAS so we use the default
     // DW_EH_PE_absptr for those.
+
+    // The section .eh_frame exhibits absolute addresses which require fixups.
+    // While the gnu linker can transform them in relative references at
+    // linking time, at the moment the mclinker does not provide this
+    // functionality. Setting the section as writable allows the fixups to be
+    // resolved at run time.
+    if (RelocM == Reloc::PIC_) {
+      eh_frame_writable = true;
+    }
     break;
   case Triple::ppc64:
   case Triple::ppc64le:

     else
       EHSectionFlags |= ELF::SHF_WRITE;
   }
+  if (eh_frame_writable) {
+    EHSectionFlags |= ELF::SHF_WRITE;
+  }
 
   // ELF
   BSSSection = Ctx->getELFSection(".bss", ELF::SHT_NOBITS,