[lld][WebAssembly] Create optional internal symbols only after LTO object as been added

[sbc100]

Summary

This is important for the cases where new symbols can be introduced
during LTO. Specifically this happens for during TLS-lowering where
references to __tls_base can be introduced.

Fixes: https://github.com/emscripten-core/emscripten/issues/12489

[sbc100]

As I mentioned in https://github.com/emscripten-core/emscripten/issues/12489 I'm having trouble reproducing this and thus creating a test for it.

The problem is that in all the tests I've managed to create all the TLS variables get lowered away by https://github.com/llvm/llvm-project/blob/f92961d238efdfeccce27f9bb7b0a6629c376d4a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp#L295.. and therefore WebAssemblyTargetLowering::LowerGlobalTLSAddress is never called.. and AFAICT that is the only reasonable place that those references to __tls_base could be created during LTO.

[tlively]

How about if you create two LTO object files, one with -mattr=+atomics,+bulk-memory and one without, then link them together? In the combined bitcode, the presence of +atomics and +bulk-memory on the functions from the first object file will prevent that stripping from running, and you should get references to __tls_base from code originating in both object files.

[sbc100]

In D111171#3043704, @tlively wrote:

How about if you create two LTO object files, one with -mattr=+atomics,+bulk-memory and one without, then link them together? In the combined bitcode, the presence of +atomics and +bulk-memory on the functions from the first object file will prevent that stripping from running, and you should get references to __tls_base from code originating in both object files.

Thanks for the tip. Yes I think I'm somehow not setting -mattr=+atomics,+bulk-memory in the LTO object file. In my test hand written .ll test case how do I enable those.. it seems not on the llvm-as command line? I guess they are embedded in the .ll itself...

[sbc100]

Finally got the test working. Fails without this change (undefined symbol __tls_base) .. passed after.

[sbc100]

• fix test

lld/test/wasm/lto/tls.ll

+; Test that LTO objects build with TLS and threading support can be linked into
+; a single threaded binary.  Specifically the references to `__tls_base` that
+; can be generated at LTO-time need to trigger the creation of the internal/fake
+; `__tls_base` symbol in the linker.
+
+; RUN: llvm-as %s -o %t1.o
+; RUN: wasm-ld --export=tls_int --export=get_tls %t1.o -o %t
+; RUN: obj2yaml %t | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1:10:20"
+target triple = "wasm32-unknown-emscripten"
+
+@tls_int = dso_local thread_local global i32 99
+
+define i32 @get_tls() #0 {
+  %val = load i32, i32* @tls_int
+  ret i32 %val
+}
+
+define void @_start() #0 {
+  ret void
+}
+
+attributes #0 = { noinline nounwind optnone "target-features"="+atomics,+bulk-memory,+mutable-globals,+sign-ext" }
+
+;      CHECK:   - Type:            GLOBAL
+; CHECK-NEXT:     Globals:
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         Type:            I32
+; CHECK-NEXT:         Mutable:         true
+; CHECK-NEXT:         InitExpr:
+; CHECK-NEXT:           Opcode:          I32_CONST
+; CHECK-NEXT:           Value:           66576
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Type:            I32
+; CHECK-NEXT:         Mutable:         false
+; CHECK-NEXT:         InitExpr:
+; CHECK-NEXT:           Opcode:          I32_CONST
+; CHECK-NEXT:           Value:           1024
+
+;      CHECK:     GlobalNames:
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         Name:            __stack_pointer
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Name:            __tls_base

lld/wasm/Driver.cpp

if (Symbol *callDtors = handleUndefined("__wasm_call_dtors")) {
         }
         WasmSym::callDtors = callDtorsFunc;
       } else {
         error("__wasm_call_dtors must be a function");
       }
     }
   }
 
-  createOptionalSymbols();
-
   if (errorCount())
     return;
 
   // Create wrapped symbols for -wrap option.
   std::vector<WrappedSymbol> wrapped = addWrappedSymbols(args);
 
   // If any of our inputs are bitcode files, the LTO code generator may create
   // references to certain library functions that might not be explicit in the
if (errorCount())
     return;
 
   // Do link-time optimization if given files are LLVM bitcode files.
   // This compiles bitcode files into real object files.
   symtab->addCombinedLTOObject();
   if (errorCount())
     return;
 
+  createOptionalSymbols();
+
   // Resolve any variant symbols that were created due to signature
   // mismatchs.
   symtab->handleSymbolVariants();
   if (errorCount())
     return;
 
   // Apply symbol renames for -wrap.
   if (!wrapped.empty())

lld/wasm/SymbolTable.cpp

DefinedGlobal *SymbolTable::addSyntheticGlobal(StringRef name, uint32_t flags,
   assert(!find(name));
   syntheticGlobals.emplace_back(global);
   return replaceSymbol<DefinedGlobal>(insertName(name).first, name, flags,
                                       nullptr, global);
 }
 
 DefinedGlobal *SymbolTable::addOptionalGlobalSymbol(StringRef name,
                                                     InputGlobal *global) {
-  LLVM_DEBUG(dbgs() << "addOptionalGlobalSymbol: " << name << " -> " << global
-                    << "\n");
   Symbol *s = find(name);
   if (!s || s->isDefined())
     return nullptr;
+  LLVM_DEBUG(dbgs() << "addOptionalGlobalSymbol: " << name << " -> " << global
+                    << "\n");
   syntheticGlobals.emplace_back(global);
   return replaceSymbol<DefinedGlobal>(s, name, WASM_SYMBOL_VISIBILITY_HIDDEN,
                                       nullptr, global);
 }
 
 DefinedTable *SymbolTable::addSyntheticTable(StringRef name, uint32_t flags,
                                              InputTable *table) {
   LLVM_DEBUG(dbgs() << "addSyntheticTable: " << name << " -> " << table