Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lld] Discard SHT_LLVM_LTO sections in relocatable links #92825

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented May 20, 2024

So long as ld -r links using bitcode always result in an ELF object, and
not a merged bitcode object, the output form a relocatable link using
FatLTO objects should not have a .llvm.lto section. Prior to this, using
the object code sections would cause the bitcode section in the output
of a relocatable link to be corrupted, by concatenating all the .llvm.lto
sections together.

This patch discards SHT_LLVM_LTO sections when not using
--fat-lto-objects, so that the relocatable ELF output won't contain
inalid bitcode.

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented May 20, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Paul Kirth (ilovepi)

Changes

So long as ld -r links using bitcode always result in an ELF object, and
not a merged bitcode object, the output form a relocatable link using
FatLTO objects should not have a .llvm.lto section. Prior to this, using
the object code sections would cause the bitcode section in the output
of a relocatable link to be corrupted, by concatenating all the .llvm.lto
sections together.

This patch discards SHT_LLVM_LTO sections when not using
--fat-lto-objects, so that the relocatable ELF output won't contain
inalid bitcode.


Full diff: https://github.com/llvm/llvm-project/pull/92825.diff

2 Files Affected:

  • (modified) lld/ELF/InputFiles.cpp (+9)
  • (modified) lld/test/ELF/fatlto/fatlto.test (+2-5)
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 1f496026d3ae2..0ac49761601c4 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -832,6 +832,15 @@ void ObjFile<ELFT>::initializeSections(bool ignoreComdats,
       this->sections[i] =
           createInputSection(i, sec, check(obj.getSectionName(sec, shstrtab)));
       break;
+    case SHT_LLVM_LTO:
+      // When doing a relocatable link with FatLTO objects, if we're not using
+      // the bitcode, discard it, since it will be concatenated together when
+      // handling orphan sections, and which will be an invalid bitcode object.
+      if (config->relocatable && !config->fatLTOObjects) {
+        sections[i] = &InputSection::discarded;
+        break;
+      }
+      LLVM_FALLTHROUGH;
     default:
       this->sections[i] =
           createInputSection(i, sec, check(obj.getSectionName(sec, shstrtab)));
diff --git a/lld/test/ELF/fatlto/fatlto.test b/lld/test/ELF/fatlto/fatlto.test
index e250325dc54f4..d2c96c3c51b98 100644
--- a/lld/test/ELF/fatlto/fatlto.test
+++ b/lld/test/ELF/fatlto/fatlto.test
@@ -50,10 +50,6 @@
 ; RUN: cmp %t/foo-fatLTO.archive %t/foo-LTO
 
 ;; Test FatLTO works with relocatable links using PIC objects
-;; Currently, with PIC relocatable links, FatLTO sections are treated as
-;; orphan sections and incorrectly concatenated together. This test verifies
-;; the current behavior, but should be fixed to either merge those sections
-;; correctly, or to drop them altogether.
 ; RUN: opt < %t/a-LTO.ll -passes="embed-bitcode<thinlto;emit-summary>" | llc --relocation-model=pic --filetype=obj -o %t/a-fat-pic.o
 ; RUN: llvm-readobj -S %t/a-fat-pic.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
 
@@ -64,9 +60,10 @@
 ; RUN: llvm-readobj -S %t/fat.pic.archive | FileCheck --check-prefix=HAS_LLVM_LTO %s
 
 ; RUN: ld.lld --whole-archive %t/fat.pic.archive -r -o %t/fat-pic-relocatable.o
-; RUN: llvm-readobj -S %t/fat-pic-relocatable.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+; RUN: llvm-readobj -S %t/fat-pic-relocatable.o | FileCheck --check-prefix=CHECK-NON-LTO-TARGET %s
 
 ; HAS_LLVM_LTO: Name: .llvm.lto
+; HAS_LLVM_LTO: Type: SHT_LLVM_LTO
 
 ;--- a-LTO.ll
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

@ilovepi ilovepi requested a review from MaskRay May 20, 2024 21:46
@MaskRay
Copy link
Member

MaskRay commented May 21, 2024

There is a bitcode wrapper (see discussions at https://reviews.llvm.org/D86847), but I agree that discarding .llvm.lto is better since concatenated .llvm.lto does not reflect the ld.lld -r <semantics-affecting-options> semantics.

sections[i] = &InputSection::discarded;
break;
}
LLVM_FALLTHROUGH;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[[fallthrough]] since C++17

@@ -832,6 +832,15 @@ void ObjFile<ELFT>::initializeSections(bool ignoreComdats,
this->sections[i] =
createInputSection(i, sec, check(obj.getSectionName(sec, shstrtab)));
break;
case SHT_LLVM_LTO:
// When doing a relocatable link with FatLTO objects, if we're not using
Copy link
Member

@MaskRay MaskRay May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Discard .llvm.lto in a relocatable link that does not use the bitcode. The concatenated output does not reflect the linking semantics. In addition, since we do not use the bitcode wrapper format, the concatenated raw bitcode would be invalid.

Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@ilovepi ilovepi changed the base branch from users/ilovepi/spr/main.lld-discard-sht_llvm_lto-sections-in-relocatable-links to main June 5, 2024 18:10
Created using spr 1.3.4
@ilovepi ilovepi merged commit 608fb46 into main Jun 8, 2024
7 checks passed
@ilovepi ilovepi deleted the users/ilovepi/spr/lld-discard-sht_llvm_lto-sections-in-relocatable-links branch June 8, 2024 00:56
nekoshirro pushed a commit to nekoshirro/Alchemist-LLVM that referenced this pull request Jun 9, 2024
So long as ld -r links using bitcode always result in an ELF object, and
not a merged bitcode object, the output form a relocatable link using
FatLTO objects should not have a .llvm.lto section. Prior to this, using
the object code sections would cause the bitcode section in the output
of a relocatable link to be corrupted, by concatenating all the
.llvm.lto
sections together.

This patch discards SHT_LLVM_LTO sections when not using
--fat-lto-objects, so that the relocatable ELF output won't contain
inalid bitcode.

Signed-off-by: Hafidz Muzakky <ais.muzakky@gmail.com>
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants