It looks like this change broke the sanitizer-x86_64-linux-autoconf buildbot.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/48824

/b/sanitizer-x86_64-linux-autoconf/build/llvm-project/compiler-rt/lib/tsan/rtl/tsan_clock.cpp:190:13: error: use of undeclared identifier 'dst'
  DCHECK_LE(dst->size_, kMaxTid);
            ^
1 error generated.

Can you please take a look?

This change broke the sanitizer-x86_64-linux bot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/26313
Please take a look.

Having given this some more thought, I'm still not in favour of this change even with start/stop symbols excluded, since there are other cases where enumeration of SHF_LINK_ORDER sections may be possible/desirable. To give one example, imagine that you have .init_array sections linked to the globals that they initialize (this may be possible if you can prove that the constructor has no side effects other than initializing the object). With this change we may for example have multiple .init_array sections for .bss and .data, which would need to be accounted for when computing DT_INIT_ARRAY/DT_INIT_ARRAY_SZ. There are various ways that you could deal with this sort of situation, but it seems like the simplest one would be to stick with one output section per file.

This change broke the msan buildbot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/39755

==70358==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5a602b2 in deleteParallelRegions /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:127:9
    #1 0x5a602b2 in (anonymous namespace)::OpenMPOpt::run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:116:16
    #2 0x5a94252 in (anonymous namespace)::OpenMPOptLegacyPass::runOnSCC(llvm::CallGraphSCC&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:559:19
    #3 0x3f53893 in RunPassOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:139:23

In D75617#1908240, @evgeny777 wrote:

@pcc Just to clarify: I'm using this patch to identify problems with WPD (typically those are either invalid reinterpret_cast or open type hierarchy). I find it convenient to obtain a list of devirtualized functions using pass remarks, "undevirtualize" some of them relink and run. With attributes it seems to be much harder

There is already a way to do this, with the attribute [[clang::lto_visibility_public]]. Why do you need another one?

This should be fixed by rG0414c5694073de26fd33a0276c47c6adea5284cf.

Now that llvm-go has been brought back, I've reverted this change.

LGTM

Does it make sense to write a test for this?

This broke users of the go bindings who were using build.sh, and changed the import path for the bindings unnecessarily. Can we please just bring back llvm-go as I requested before?

We should bring back llvm-go instead. That program doesn't depend on llgo, it's mostly a helper for building the Go bindings.

• Adjust the test to use MaxSize-64

PTAL

Finalize

• Zero initialize array

• Use heap

LGTM

LGTM

LGTM

New approach

LGTM

LGTM

LGTM

LGTM

I am suggesting that you wouldn't change this function at all but would hash getSourceFileName yourself. (You shouldn't use getModuleIdentifier for anything that affects the contents of the output file because it isn't serialized.)

If you don't need a guarantee of uniqueness wouldn't it be better to just take a hash of Module::getSourceFileName() instead of looking at the names of the globals?

In D73143#1832560, @eugenis wrote:

Fallback to a different allocator is hard, because then you need to implement PointerIsMine to redirect to the right allocator in free(), and AFAIK we don't have that in scudo for the larger allocations (i.e. secondary).

On Aarch64, right now only CALL26 and JUMP26 instructions generate PLT relocations, so we manifest them with stubs that are just jumps to the original function.

2. Add a non-GC-root dummy section. This is a bit awkward to implement in MC because we have to pick a section name. The section may be combined with other normal sections with the same name. We need to be careful and not cause an "incompatible section flags" error (lld specific).

LGTM

If we just want a quick fix then I would prefer if we consider strong hidden symbols to be non-preemptible. This just seems like it would cause our assembler to hide bugs of the sort that D72197 was intended to catch, while changing the behavior for hidden symbols would mean that we continue to catch those bugs.

Shouldn't we just implement support for the relocation instead?

In D72197#1825247, @MaskRay wrote:

In D72197#1825012, @pcc wrote:

It looks like this change caused us to start rejecting:

.thumb
.thumb_func
.globl foo
.hidden foo
foo:
adr lr, foo + 1

with:

test.s:6:1: error: unsupported relocation on symbol
adr lr, foo + 1
^

This is exposed by this Android ART code: https://cs.android.com/android/platform/superproject/+/master:art/runtime/arch/arm/quick_entrypoints_arm.S;l=1826

I see a couple of possible fixes for this:

1. We could go back to the previous behaviour for global hidden symbols.
2. We could teach MC and LLD about the R_ARM_THM_ALU_PREL_11_0 relocation required to relocate this instruction. Arguably the ART code shouldn't be adding the 1 for Thumb here (because R_ARM_THM_ALU_PREL_11_0 adds the 1 itself), so ART would then need to be fixed.

foo is a hidden definition in the current object file. Can the ART code be changed to use a local symbol (adr lr, .Lfoo) instead?

In D65653#1825141, @efriedma wrote:

Exception handling has to use a DWARF unwinder, not a "fast" unwinder. So the "fast" unwinder can't actually do anything with the address other than print it out.

In D72197#1825070, @MaskRay wrote:

In D72197#1825012, @pcc wrote:

It looks like this change caused us to start rejecting:

.thumb
.thumb_func
.globl foo
.hidden foo
foo:
adr lr, foo + 1

with:

test.s:6:1: error: unsupported relocation on symbol
adr lr, foo + 1
^

This is exposed by this Android ART code: https://cs.android.com/android/platform/superproject/+/master:art/runtime/arch/arm/quick_entrypoints_arm.S;l=1826

I see a couple of possible fixes for this:

1. We could go back to the previous behaviour for global hidden symbols.
2. We could teach MC and LLD about the R_ARM_THM_ALU_PREL_11_0 relocation required to relocate this instruction. Arguably the ART code shouldn't be adding the 1 for Thumb here (because R_ARM_THM_ALU_PREL_11_0 adds the 1 itself), so ART would then need to be fixed.

% arm-linux-gnueabi-as /tmp/c/a.s -o /tmp/c/a.o
/tmp/c/a.s: Assembler messages:
/tmp/c/a.s:6: Error: invalid Hi register with immediate
/tmp/c/a.s:6: Error: invalid immediate for address calculation (value = 0xFFFFFFFFFFFFFFFE)

How can I make GNU as recognize it?