[PATCH] D97436: [lld-link] Fix addrsig symbols merging in ICF.

Fri Feb 26 21:12:12 PST 2021

pcc added a comment.

In D97436#2592053 <https://reviews.llvm.org/D97436#2592053>, @zequanwu wrote:

> In D97436#2591761 <https://reviews.llvm.org/D97436#2591761>, @pcc wrote:
>
>> As far as I know the current behavior is that `/opt:icf` gives you full ICF for executable sections and safe ICF for non-executable sections. This is more than what MSVC does, which is full ICF for executable sections and no ICF for non-executable sections. I think it makes sense to add a mode that gives you full ICF for non-executable sections, but the behavior of `/opt:icf` should be preserved.
>
> Initially, we found <https://bugs.chromium.org/p/chromium/issues/detail?id=1169276> that the current behavior of `/opt:icf` merges executable sections even if they are in address-significant table, which obviously is a bug.

No it isn't. This matches the behavior of MSVC, which doesn't have a concept of an address significance table.

> This patch is trying to fix it and adding a `/opt:safeicf` to do safe icf for both executable and non-executable sections. The behavior of `/opt:icf` is preserved.

If you think that `/opt:icf` should do safe ICF for executable sections then I don't get what you think the difference between `/opt:icf` and `/opt:safeicf` should be.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97436/new/

https://reviews.llvm.org/D97436