Reland D64327 [MC][ELF] Allow STT_SECTION referencing SHF_MERGE on REL targets
authorFangrui Song <i@maskray.me>
Mon, 3 Aug 2020 01:05:17 +0000 (18:05 -0700)
committerFangrui Song <i@maskray.me>
Mon, 3 Aug 2020 01:05:17 +0000 (18:05 -0700)
This drops a GNU gold workaround and reverts the revert commit rL366708.

  Before binutils 2.34, gold -O2 and above did not correctly handle R_386_GOTOFF to
  SHF_MERGE|SHF_STRINGS sections: https://sourceware.org/bugzilla/show_bug.cgi?id=16794

From the original review:

  ... it reduced the size of a big ARM-32 debug image by 33%. It contained ~68M
  of relocations symbols out of total ~71M symbols (96% of symbols table was
  generated for relocations with symbol).

-Wl,-O2 (and -Wl,-O3) is so rare that we should just lower the
optimization level for LLVM_LINKER_IS_GOLD rather than pessimizing all users.

llvm/cmake/modules/AddLLVM.cmake
llvm/lib/MC/ELFObjectWriter.cpp
llvm/test/MC/ELF/basic-elf-32.s
llvm/test/MC/ELF/compression.s
llvm/test/MC/ELF/relocation-386.s
llvm/test/MC/Mips/elf-relsym.s
llvm/test/MC/Mips/xgot.s

index 333167b..a25f3e8 100644 (file)
@@ -222,8 +222,13 @@ function(add_link_opts target_name)
     # Pass -O3 to the linker. This enabled different optimizations on different
     # linkers.
     if(NOT (${CMAKE_SYSTEM_NAME} MATCHES "Darwin|SunOS|AIX" OR WIN32))
-      set_property(TARGET ${target_name} APPEND_STRING PROPERTY
-                   LINK_FLAGS " -Wl,-O3")
+      # Before binutils 2.34, gold -O2 and above did not correctly handle R_386_GOTOFF to
+      # SHF_MERGE|SHF_STRINGS sections: https://sourceware.org/bugzilla/show_bug.cgi?id=16794
+      if(LLVM_LINKER_IS_GOLD)
+        set_property(TARGET ${target_name} APPEND_STRING PROPERTY LINK_FLAGS " -Wl,-O1")
+      else()
+        set_property(TARGET ${target_name} APPEND_STRING PROPERTY LINK_FLAGS " -Wl,-O3")
+      endif()
     endif()
 
     if(LLVM_LINKER_IS_GOLD)
index 1ca9d0f..5a5692c 100644 (file)
@@ -1389,11 +1389,6 @@ bool ELFObjectWriter::shouldRelocateWithSymbol(const MCAssembler &Asm,
     if (Flags & ELF::SHF_MERGE) {
       if (C != 0)
         return true;
-
-      // It looks like gold has a bug (http://sourceware.org/PR16794) and can
-      // only handle section relocations to mergeable sections if using RELA.
-      if (!hasRelocationAddend())
-        return true;
     }
 
     // Most TLS relocations use a got, so they need the symbol. Even those that
index effc363..1a92310 100644 (file)
@@ -46,9 +46,9 @@ main:                                   # @main
 
 // CHECK: Relocations [
 // CHECK:   Section {{.*}} .rel.text {
-// CHECK:     0x6  R_386_32   .L.str1
+// CHECK:     0x6  R_386_32   .rodata.str1.1 0x0
 // CHECK:     0xB  R_386_PC32 puts
-// CHECK:     0x12 R_386_32   .L.str2
+// CHECK:     0x12 R_386_32   .rodata.str1.1 0x0
 // CHECK:     0x17 R_386_PC32 puts
 // CHECK:   }
 // CHECK: ]
index 93a77b3..9bc5c0e 100644 (file)
@@ -38,7 +38,7 @@
 
 // In x86 32 bit named symbols are used for temporary symbols in merge
 // sections, so make sure we handle symbols inside compressed sections
-// 386-SYMBOLS-GNU: Name: .Linfo_string0
+// 386-SYMBOLS-GNU: Name: .zdebug_str
 // 386-SYMBOLS-GNU-NOT: }
 // 386-SYMBOLS-GNU: Section: .zdebug_str
 
@@ -73,7 +73,7 @@
 // ZLIB-STYLE-FLAGS32-NEXT: AddressAlignment: 4
 // ZLIB-STYLE-FLAGS64-NEXT: AddressAlignment: 8
 
-// 386-SYMBOLS-ZLIB: Name: .Linfo_string0
+// 386-SYMBOLS-ZLIB: Name: .debug_str
 // 386-SYMBOLS-ZLIB-NOT: }
 // 386-SYMBOLS-ZLIB: Section: .debug_str
 
index 4273750..b9bf9b6 100644 (file)
@@ -8,7 +8,7 @@
 // I386: Format: elf32-i386
 // CHECK:      Relocations [
 // CHECK-NEXT:   Section {{.*}} .rel.text {
-// CHECK-NEXT:     0x2          R_386_GOTOFF     .Lfoo 0x0
+// CHECK-NEXT:     0x2          R_386_GOTOFF     .rodata.str1.16 0x0
 // CHECK-NEXT:     0x{{[^ ]+}}  R_386_PLT32      bar2 0x0
 // CHECK-NEXT:     0x{{[^ ]+}}  R_386_GOTPC      _GLOBAL_OFFSET_TABLE_ 0x0
 // Relocation 3 (bar3@GOTOFF) is done with symbol 7 (bss)
index b8c2f89..d19b4e3 100644 (file)
@@ -4,16 +4,10 @@
 
 // CHECK: Symbols [
 // CHECK:   Symbol {
-// CHECK:     Name: $.str
+// CHECK:     Name: .rodata.cst8
 // CHECK:   }
 // CHECK:   Symbol {
-// CHECK:     Name: $.str1
-// CHECK:   }
-// CHECK:   Symbol {
-// CHECK:     Name: $CPI0_0
-// CHECK:   }
-// CHECK:   Symbol {
-// CHECK:     Name: $CPI0_1
+// CHECK:     Name: .rodata.str1.1
 // CHECK:   }
 // CHECK: ]
 
index 100d25e..76490f0 100644 (file)
@@ -10,8 +10,8 @@
 // CHECK:   0x1C R_MIPS_GOT_LO16 ext_1
 // CHECK:   0x24 R_MIPS_CALL_HI16 printf
 // CHECK:   0x30 R_MIPS_CALL_LO16 printf
-// CHECK:   0x2C R_MIPS_GOT16 $.str
-// CHECK:   0x38 R_MIPS_LO16 $.str
+// CHECK:   0x2C R_MIPS_GOT16 .rodata.str1.1 0x0
+// CHECK:   0x38 R_MIPS_LO16 .rodata.str1.1 0x0
 // CHECK: ]
 
        .text