[PATCH] D41060: Add an option for ICFing data

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama <ruiu at google.com> writes:

> On Mon, Dec 11, 2017 at 10:02 AM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>>
>> > ruiu added a comment.
>> >
>> > We used to merge data for COFF but not anymore because it causes nasty
>> problems that are hard to find and debug. E.g.
>> https://bugs.chromium.org/p/chromium/issues/detail?id=682773#c24
>> >
>> > Are you sure you don't have that kind of issue?
>>
>> It is an explicit command line option, but some users do enable it.
>>
>
> Yeah, I know, but I wonder if you could actually link your program without
> any hidden run-time errors. We've been using lld with such configuration
> that all read-only data would be merged, and we didn't notice any error at
> first. From that experience, I'm worried that this feature can be quite
> dangerous which looks benign at first but actually creates hidden bugs. If
> users including you really want this, I wouldn't stop thought.

They are using the option in the current linker. I guess the main
difference is that they have been using a linker that has this feature
during the entire development.

Cheers,
Rafael