Skip to content

Commit

Permalink
ELF ICF: Merge only functions.
Browse files Browse the repository at this point in the history
Previously, LLD merged all read-only sections. So the following
program prints out "true" if -icf=all is specified.

  static const int foo = 1;
  static const int bar = 1;
  int main() { printf("%s\n", &foo == &bar ? "true" : "false"); }

This is somewhat counter-intuitive, and it actually caused nasty issues.
One example is https://bugs.chromium.org/p/chromium/issues/detail?id=682773#c24.

This patch changes the way how it works. Now ICF merges only functions
(i.e. executable sections).

Differential Revision: https://reviews.llvm.org/D30365

llvm-svn: 296534
  • Loading branch information
rui314 committed Feb 28, 2017
1 parent 8dabd7e commit 13ed0b6
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lld/ELF/ICF.cpp
Expand Up @@ -162,8 +162,8 @@ static bool isEligible(InputSection *S) {
// .init and .fini contains instructions that must be executed to
// initialize and finalize the process. They cannot and should not
// be merged.
return S->Live && (S->Flags & SHF_ALLOC) && !(S->Flags & SHF_WRITE) &&
S->Name != ".init" && S->Name != ".fini";
return S->Live && (S->Flags & SHF_ALLOC) && (S->Flags & SHF_EXECINSTR) &&
!(S->Flags & SHF_WRITE) && S->Name != ".init" && S->Name != ".fini";
}

// Split an equivalence class into smaller classes.
Expand Down
20 changes: 20 additions & 0 deletions lld/test/ELF/icf9.s
@@ -0,0 +1,20 @@
# REQUIRES: x86

### Make sure that we do not merge data.
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
# RUN: ld.lld %t -o %t2 --icf=all --verbose | FileCheck %s

# CHECK-NOT: selected .data.d1
# CHECK-NOT: selected .data.d2

.globl _start, d1, d2
_start:
ret

.section .data.f1, "a"
d1:
.byte 1

.section .data.f2, "a"
d2:
.byte 1

0 comments on commit 13ed0b6

Please sign in to comment.