[ELF][ARM] Process ARM build attributes of dynamic libraries.

[ikudrin]

Summary

This helps to detect incompatible settings between dynamic libraries and other input files.

Note that GOLD issues an error in that case, while LD ignores it.
However, I believe that it is better to be strict here.

[grimar]

ELF/InputFiles.cpp

591

We have Config->IsLE.

1005

Can we have ARMAttributesSec when Config->EMachine != EM_ARM?
Seems it can be just if (this->ARMAttributesSec)

[peter.smith]

I have several reservations about doing this:

• BuildAttributes are only defined for relocatable object files, the ABI does not require or prevent them from appearing in Shared Objects or Executables therefore nothing should rely on their presence.
• Can it be guaranteed that the shared object we link against will have the same attributes as the shared object at run time?
• Should the attributes of the shared object be aggregated with the attributes from the relocatable object?

This is all outside of the specification http://infocenter.arm.com/help/topic/com.arm.doc.ihi0045e/IHI0045E_ABI_addenda.pdf BuildAttributes are only defined for relocatable objects see (2.1.3 The scope of build attributes). Granted there is nothing stopping LLD defining its own behaviour on top of the specification but I think we should be very careful doing so. Given that we can't guarantee what we are linking against dynamically I'm definitely against updating the attributes of the ELF file based on Shared Objects, at least not without proposing a change to the ABI. There may be a case for a warning for certain specific clashes, but I don't think we should do this in the general case.

I'm out of the office till next year so I can't give many more comments on this. There is a mailing list for the Arm ABI [e-mail] that you can ask further questions on what BuildAttributes should be used for.

[ikudrin]

Thank you for the explanation, Peter!

It seems like we definitely should not update the attributes based on Shared Objects, but what do you think about preventing linking against SOs with clashing attributes? Should we prevent that and issue an error (like GOLD does), or ignore it (like LD), or, maybe, just warn a user?

[peter.smith]

Unless we have evidence that many users are making the mistake of linking against incompatible shared objects, my personal preference is that it is better to follow ld.bfd and ignore. I think most toolchains and distributions make this mistake hard to do by accident.

If we do have evidence that this is a problem, or others feel strongly about it, then my personal preference is a warning. For example a function in a shared library might have uses hard-float in its BuildAttributes but no floating-point types in its interface so it is callable from both a hard-float and soft-float ABI executable. This is in theory true of relocatable objects as well but the interfaces to shared libraries tend to be narrower and more feasible to control.

[ikudrin]

Thanks again, Peter!

ELF/InputFiles.h

template <class ELFT> class SharedFile : public ELFFileBase<ELFT> {
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::SymRange Elf_Sym_Range;
   typedef typename ELFT::Verdef Elf_Verdef;
   typedef typename ELFT::Versym Elf_Versym;
 
   const Elf_Shdr *VersymSec = nullptr;
   const Elf_Shdr *VerdefSec = nullptr;
+  const Elf_Shdr *ARMAttributesSec = nullptr;
 
 public:
   std::vector<const Elf_Verdef *> Verdefs;
   std::string SoName;
 
   static bool classof(const InputFile *F) {
     return F->kind() == Base::SharedKind;
   }

ELF/InputFiles.cpp

if (Arch != ARMBuildAttrs::v6_M && Arch != ARMBuildAttrs::v6S_M)
       // All Architectures used in Cortex processors with the exception
       // of v6-M and v6S-M have the MOVT and MOVW instructions.
       Config->ARMHasMovtMovw = true;
     break;
   }
 }
 
 template <class ELFT>
+static void processARMAttributesSection(const ELFFileBase<ELFT> *F,
+                                        const typename ELFT::Shdr &Sec) {
+  ARMAttributeParser Attributes;
+  ArrayRef<uint8_t> Contents = check(F->getObj().getSectionContents(&Sec));
+  Attributes.Parse(Contents, /*isLittle*/ Config->EKind == ELF32LEKind);
+  updateSupportedARMFeatures(Attributes);
+  updateARMVFPArgs(Attributes, F);
+}
+
+template <class ELFT>
 InputSectionBase *ObjFile<ELFT>::getRelocTarget(const Elf_Shdr &Sec) {
   uint32_t Idx = Sec.sh_info;
   if (Idx >= this->Sections.size())
     fatal(toString(this) + ": invalid relocated section index: " + Twine(Idx));
   InputSectionBase *Target = this->Sections[Idx];
 
   // Strictly speaking, a relocation section must be included in the
   // group of the section it relocates. However, LLVM 3.3 and earlier

 template <class ELFT>
 InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   StringRef Name = getSectionName(Sec);
 
   switch (Sec.sh_type) {
   case SHT_ARM_ATTRIBUTES: {
     if (Config->EMachine != EM_ARM)
       break;
-    ARMAttributeParser Attributes;
-    ArrayRef<uint8_t> Contents = check(this->getObj().getSectionContents(&Sec));
-    Attributes.Parse(Contents, /*isLittle*/ Config->EKind == ELF32LEKind);
-    updateSupportedARMFeatures(Attributes);
-    updateARMVFPArgs(Attributes, this);
+    processARMAttributesSection(this, Sec);
 
     // FIXME: Retain the first attribute section we see. The eglibc ARM
     // dynamic loaders require the presence of an attribute section for dlopen
     // to work. In a full implementation we would merge all attribute sections.
     if (In.ARMAttributes == nullptr) {
       In.ARMAttributes = make<InputSection>(*this, Sec, Name);
       return In.ARMAttributes;
     }
template <class ELFT> void SharedFile<ELFT>::parseSoName() {
   // Search for .dynsym, .dynamic, .symtab, .gnu.version and .gnu.version_d.
   for (const Elf_Shdr &Sec : Sections) {
     switch (Sec.sh_type) {
     default:
       continue;
     case SHT_DYNSYM:
       this->initSymtab(Sections, &Sec);
       break;
+    case SHT_ARM_ATTRIBUTES:
+      this->ARMAttributesSec = &Sec;
+      break;
     case SHT_DYNAMIC:
       DynamicSec = &Sec;
       break;
     case SHT_SYMTAB_SHNDX:
       this->SymtabSHNDX = CHECK(Obj.getSHNDXTable(Sec, Sections), this);
       break;
     case SHT_GNU_versym:
       this->VersymSec = &Sec;

 // The special index 0 is reserved for VERF_NDX_LOCAL and 1 is for
 // VER_NDX_GLOBAL. There's no table entry for these special versions in
 // ".gnu.version_d".
 //
 // The file format for symbol versioning is perhaps a bit more complicated
 // than necessary, but you can easily understand the code if you wrap your
 // head around the data structure described above.
 template <class ELFT> void SharedFile<ELFT>::parseRest() {
+  if (this->ARMAttributesSec && Config->EMachine == EM_ARM)
+    processARMAttributesSection(this, *(this->ARMAttributesSec));
   Verdefs = parseVerdefs();                       // parse .gnu.version_d
   std::vector<uint32_t> Versyms = parseVersyms(); // parse .gnu.version
   ArrayRef<Elf_Shdr> Sections = CHECK(this->getObj().sections(), this);
 
   // System libraries can have a lot of symbols with versions. Using a
   // fixed buffer for computing the versions name (foo@ver) can save a
   // lot of allocations.
   SmallString<0> VersionedNameBuffer;

test/ELF/arm-tag-vfp-args-errs.s

 // REQUIRES:arm
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %S/Inputs/arm-vfp-arg-base.s -o %tbase.o
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %S/Inputs/arm-vfp-arg-vfp.s -o %tvfp.o
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %S/Inputs/arm-vfp-arg-toolchain.s -o %ttoolchain.o
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.o
+// RUN: ld.lld -shared %tbase.o -o %tbase.so
+// RUN: ld.lld -shared %tvfp.o -o %tvfp.so
 // RUN: not ld.lld %t.o %tbase.o %tvfp.o -o%t 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o %tbase.o %ttoolchain.o -o%t 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o %tvfp.o %tbase.o -o%t 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o %tvfp.o %ttoolchain.o -o%t 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o %ttoolchain.o %tbase.o -o%t 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o %ttoolchain.o %tvfp.o -o%t 2>&1 | FileCheck %s
+// RUN: not ld.lld %t.o %tbase.o %tvfp.so -o%t 2>&1 | FileCheck %s
+// RUN: not ld.lld %t.o %tbase.so %tvfp.o -o%t 2>&1 | FileCheck %s
 
 // CHECK: incompatible Tag_ABI_VFP_args
   .arch armv7-a
   .eabi_attribute 20, 1
   .eabi_attribute 21, 1
   .eabi_attribute 23, 3
   .eabi_attribute 24, 1
   .eabi_attribute 25, 1