Thanks @MaskRay and @peter.smith for investigating this problem, I can confirm that this change fixes it for me.

Hi @MaskRay, it looks like this change has caused some binaries to start segfaulting on Android aarch64 devices. The strange thing is that the segfault happens even before entering the dynamic loader, and furthermore the segfault doesn't happen if I invoke the dynamic loader directly, so I'm guessing that the Linux kernel doesn't like the binary for some reason.

$ ~/l2/ra/bin/ld.lld @response.txt
$ adb -s $pixel2 push bin /data/local/tmp
bin: 1 file pushed. 3.9 MB/s (15608 bytes in 0.004s)
$ adb -s $pixel2 shell
walleye:/ # /data/local/tmp/bin
Segmentation fault 
139|walleye:/ # /system/bin/linker64 /data/local/tmp/bin                                                                                   
CANNOT LINK EXECUTABLE "/data/local/tmp/bin": library "libclang_rt.hwasan-aarch64-android.so" not found

Without this change:

$ ~/l/ra/bin/ld.lld @response.txt
$ adb -s $pixel2 push bin /data/local/tmp
bin: 1 file pushed. 1.2 MB/s (17992 bytes in 0.014s)
$ adb -s $pixel2 shell
walleye:/ # /data/local/tmp/bin
CANNOT LINK EXECUTABLE "/data/local/tmp/bin": library "libclang_rt.hwasan-aarch64-android.so" not found

These instructions use Android adb, but given that the problem appears to be with the Linux kernel you might be able to reproduce on regular Linux as well.

• Support aliases and other constants

In D66377#1642092, @eugenis wrote:

What's the binary size overhead? I assume most of it comes from adding personality functions to noexcept but !nounwind functions?

Ping.

Ping.

LGTM

This would also be fixed by switching to canonical aliases, right? Maybe another data point in favour of finally switching.

In D63932#1610182, @ostannard wrote:

Partial linking will indeed prevent dropping the virtual functions, but it should not prevent clearing the pointer to the virtual function in the vtable. The linker should then be able to drop the virtual function body as part of --gc-sections during the final link.

If partial linking isn't doing internalisation, I'd expect that to prevent a lot of other LTO optimisations, not just VFE. Is this a common use-case which we need to care about?

LGTM

LGTM

LGTM

LGTM

HWASAN should only be using GOT relative relocations to access shadow memory, so I wouldn't expect this change to have an impact on HWASAN.

• Address review comments

Thanks for the confirmation Kees.

• Switch to a symbol table lookup
• Use --implicit-check-not

In D65857#1620933, @peter.smith wrote:

In D65857#1619366, @pcc wrote:

MHO: The assembler is a low enough level component that the user can be presumed to know what they're doing, regardless of linker limitations. So I would prefer not to do this. If we do anything about this, we should document the limitations of the GNU linkers somewhere.

It is a tricky balance, we don't want to rule out a reasonable use case, but ideally we want to detect problems as soon as possible. I think what I have here may be overly strict as if you happen to know that the result of a signed operation is positive then this can work, or if you happen to know you are linking with LLD.

One possible compromise is some kind of strict mode, something like --strict-movw-relocs that people could enable to restrict the relocations to a GNU compatible subset for those that need it. The command line option could also act as a kind of documentation that is a bit more visible. Any thoughts?

In D65478#1620027, @rahmanl wrote:

In D65478#1620005, @pcc wrote:

In D65478#1619990, @rahmanl wrote:

In D65478#1619713, @inglorion wrote:

Unfortunately, this causes some problems. It looks like the sections are created with different attributes than the original. This causes problems such as sections being writable when the original was read-only, including https://crbug.com/990942 where that causes program crashes. There is some more discussion on that bug report.

I'll revert this to unbreak the Chromium build, and we can come up with a way forward from there.

Hi Bob,
We plan to gate this feature behind a condition: D65837
I think in the Chromium case, what it needs is the -funique-section-names. More precisely, Chromium needs this flag only for the explicitly defined section "protected_memory", but I don't think it hurts to do it globally ( Chromium build may actually be using this flag anyways. Please confirm.)
If you think that the chromium build could use this flag, I will change D65837 to do the check.

Chromium is depending on an implementation detail here and the revert is temporary until we can land a compiler feature that will let us avoid the implementation detail dependence. It doesn't seem necessary to change the condition that you've already implemented in D65837.

Agreed.
However, I was thinking that the implementation detail relies on the assumption that only one section exists of the name "protected_memory" and if we somehow let the programmer specify that a particular section name can only appear once, it would resolve the problem. Enforcing the use of funique-section-names does look overkill.

• Add test

In D65478#1619990, @rahmanl wrote:

In D65478#1619713, @inglorion wrote:

Unfortunately, this causes some problems. It looks like the sections are created with different attributes than the original. This causes problems such as sections being writable when the original was read-only, including https://crbug.com/990942 where that causes program crashes. There is some more discussion on that bug report.

I'll revert this to unbreak the Chromium build, and we can come up with a way forward from there.

Hi Bob,
We plan to gate this feature behind a condition: D65837
I think in the Chromium case, what it needs is the -funique-section-names. More precisely, Chromium needs this flag only for the explicitly defined section "protected_memory", but I don't think it hurts to do it globally ( Chromium build may actually be using this flag anyways. Please confirm.)
If you think that the chromium build could use this flag, I will change D65837 to do the check.

MHO: The assembler is a low enough level component that the user can be presumed to know what they're doing, regardless of linker limitations. So I would prefer not to do this. If we do anything about this, we should document the limitations of the GNU linkers somewhere.

In rL367501#677263, @rahmanl wrote:

I'm not sure the second necessarily follows from the first. The way unique section names work for function-sections is to use a dot-separated suffix (".text._Z3foo", etc). Is the "strip the dot-separated suffix" standard, or some special case for .text? If it's standardized in some way, we could rely on that - adding a suffix to explicitly-named sections & the linker would remove those.

Yes, Unfortunately, section coalescing apparently only works with predefined sections (.text .data etc.).

@pcc had some thoughts on this too, his preference was more towards (2), so hopefully he'll chime in on my perspectives here so we can hash out the details.

I think It is really the use of unique section ids which makes the intended feature of this CL possible. So, if integrated-as=true entails that unique section ids are available, then we should condition on that.