Fix Thumb text sections' flags under COFF/Windows (2/2)

[aguinet]

Summary

This is a follow up of D21662.

The main issue here is that the "thumb" flag wasn't set for some of these
sections, making MSVC's link.exe fails to correctly relocate code
against the symbols inside these sections. link.exe could fail for
instance with the "fixup is not aligned for target 'XX'" error. If
linking doesn't fail, the relocation process goes wrong in the end and
invalid code is generated by the linker.

To reproduce the error, one can compile this file as a static library:

#include <stdio.h>

void test(const char* s)
{
const void* ptr = &snprintf;
printf("%s: %p\n", s, ptr);
}

and then create a dynamic library that links against this static library
(which calls the 'test' function). A visual studio project can be
provided for people interested.

[compnerd]

Nice find on this. Modulo the comment/style things, this LGTM.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

825

The same thing applies to lld ;-). This is not link.exe specific.

The flag isn't "basically" telling the linker, it is exactly telling the linker that. This is a carry over from the older ARM support: thumb(-1) was 16-bit instructions, and the IMAGE_SCN_MEM_16BIT indicates that the text is 16-bit instructions, aka thumb mode.

I don't think that the comment adds anything of value.

828

Kill the unnecessary braces.

I think you can write this more compactly by doing:

| (TM.getTargetTriple().getArch() == Triple::thumb ? COFF::IMAGE_SCN_MEM_16BIT : 0)

[rengolin]

With Saleem's comments. LGTM.

[rengolin]

r273880

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 //===----------------------------------------------------------------------===//
 
 static unsigned
-getCOFFSectionFlags(SectionKind K) {
+getCOFFSectionFlags(SectionKind K, const TargetMachine &TM) {
   unsigned Flags = 0;
 
   if (K.isMetadata())
     Flags |=
       COFF::IMAGE_SCN_MEM_DISCARDABLE;
-  else if (K.isText())
+  else if (K.isText()) {
     Flags |=
       COFF::IMAGE_SCN_MEM_EXECUTE |
       COFF::IMAGE_SCN_MEM_READ |
       COFF::IMAGE_SCN_CNT_CODE;
+  // This basically tells link.exe that this text section contains thumb
+  // code. See comments in MCObjectFileInfo::initCOFFMCObjectFileInfo (in
+  // MC/MCObjectFileInfo.cpp) for a more detailed explaination.
+    if (TM.getTargetTriple().getArch() == Triple::thumb) {
+      Flags |= COFF::IMAGE_SCN_MEM_16BIT;
+    }
+  }
   else if (K.isBSS())
     Flags |=
       COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA |

     const GlobalValue *GV, SectionKind Kind, Mangler &Mang,
     const TargetMachine &TM) const {
   int Selection = 0;
-  unsigned Characteristics = getCOFFSectionFlags(Kind);
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
   StringRef Name = GV->getSection();
   StringRef COMDATSymName = "";
   if (GV->hasComdat()) {

 
   if ((EmitUniquedSection && !Kind.isCommon()) || GV->hasComdat()) {
     const char *Name = getCOFFSectionNameForUniqueGlobal(Kind);
-    unsigned Characteristics = getCOFFSectionFlags(Kind);
+    unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
 
     Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
     int Selection = getSelectionForCOFF(GV);

 
   SectionKind Kind = SectionKind::getReadOnly();
   const char *Name = getCOFFSectionNameForUniqueGlobal(Kind);
-  unsigned Characteristics = getCOFFSectionFlags(Kind);
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
   Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
   unsigned UniqueID = NextUniqueID++;