In D56672#1506944, @evgeny777 wrote:

Could this use a relative relocation so that we aren't paying the cost of a dynamic relocation for each global?

@pcc Sorry I missed your comment

I think this is not possible to do for shared objects, because GVs could be preempted.

I think it should be possible if you make the relocation point to the object itself (which will have local linkage) and not the alias that you're creating. Of course you'll need to add the tag to the address in the same way as when you create the alias.

Still this looks possible for executables where I can set
visibility of instrumented globals to hidden. I'll try this out

Ping^2.

LGTM

I think this would need to be clearly labeled as an experimental builder so that it's clear that developers aren't on the hook for fixing it. It also means that the bot will need to be configured not to send mail to the blamelist.

• Use a one-liner to initialize Partitions

Sorry for the long delay in reviewing this.

I think there might still be a problem where we're dropping the distinction between visible to other translation units and visible outside the linkage unit. This is related to the distinction between LDPR_PREVAILING_DEF_IRONLY and LDPR_PREVAILING_DEF_IRONLY_EXP in the gold plugin interface. Let me look into it a bit more and get back to you.

• Switch to using llvm-readelf

• Rebase
• Address review comments

• Add a note about the limit on the number of partitions

Ping.

Could you please revert the changes from D54815 out of this change so that it is easier to read?

• Adjust error message format

Interesting, it looks like the difference here is almost entirely due to different inlining decisions.

• Use std::vector

Adding some more people who may be able to review the LLVM changes here.

We shouldn't be compiling any of the sanitizer runtimes with sanitizers, that's pretty much guaranteed not to work.

I probably wouldn't do anything about suppressing init on alloca for now, but if we did do something I think I'd be in favour of the separate builtin for uninitialized alloca. I also considered the alternative of allowing __attribute__((uninitialized)) to appear on a function or block statement, but it seemed clearer for the uninitialized-ness to be as close to the alloca as possible.

LGTM as well

• Add missing pieces to MC; add test

• Split out code into functions

• Add comment

In D60242#1456022, @bd1976llvm wrote:

Hi Peter,

I really like the concept. I had some nebulous concerns when I read the RFC; but, I didn't comment as I couldn't think of any alternatives to what you were proposing. What I am writing here is still not fully formed.. so feel free to ignore if it is unhelpful.

My main concern is that this is supporting a rare usecase and I don't really like the idea of adding complexity to the core tools for niche cases.

I feel like it would be better if this could be implemented via post-processing in some way; outside of the core tools.

Some aspect of this, like the linker being able to report all of the functions reachable from a given set of entry points, are of general utility though.

I wonder if there is any scope for implementing something different along the lines of:

1. Do an initial link.
2. Analyse the resulting elf and report all of the functions in each partition.
3. Do another link but this time provide an ordering file so that all of the functions for each partition are placed contiguously.
4. Extract the functions for each partition, apart form the first, into a partition file which also contains metadata specifying the address range.
5. Write zeros over all of the functions apart from those in the main partition and then compress the executable (saving the file space).
6. A loader extension patches in the missing functions for each partition when needed.

• Address review comments

In D60226#1455477, @tejohnson wrote:

In D60226#1453856, @steven_wu wrote:

Also do note, I did a quick benchmark on thin-link time for libLTO.dylib. thin-link was 2.6s before the change vs. 4.0s after the change. Building IRSymtab during thin-link does introduce non-negligible overhead. It might be possible to build them async in threads to reduce the overhead.

@pcc is this surprising? Typically we should just be loading it out of the bitcode file. There are cases where it has to be rebuilt, but theoretically those should be exceptions.

In D60059#1450320, @lebedev.ri wrote:

In D60059#1450252, @yuxuanchen1997 wrote:

enable all runtime sanitizers for extra safety

Brilliant idea. But I wonder how that could be done for -fsanitize. ASAN and MSAN can't be used together.

Nonsense.
There is a patch that allows to offload the shadow memory onto the cloud via blockchain,
in encrypted form for extra security of course. That will allow to use both sanitizers together.

In D60026#1449098, @peter.smith wrote:

I'm happy to make the change. One minor reservation is that from combineEHSections to ARMExidxSections::finalize() the section will report its size as 0 and each individual ARMExidx InputSection won't have a parent. To the best of my knowledge this doesn't matter right now as nothing depends on these being set during that time. It may give someone a surprise in the future though.