[lld][WebAssembly] Don't allow `--global-base` to be specified in -share/-pie or --relocatable modes

[sbc100]

Summary

Add some checks around this combination of flags

Also, honor --global-base when specified in --stack-first mode
rather than ignoring it. But error out if the specified base preseeds
the end of the stack.

[sunfish]

lld/wasm/Writer.cpp

269

Could you add a comment here explaining why this is 1024? I think it's to avoid putting any global at address 0 which C/C++ would consider to be a null pointer, and 1024 is a nice round (in binary) number. Is there any other consideration here?

[sbc100]

lld/wasm/Writer.cpp

269

yes, I agree we should document. Note that this change doesn't change the default value though, it just shifts it from being set in one place to another. In the old code the default value of 1024 was in Driver.cpp: config->globalBase = args::getInteger(args, OPT_global_base, 1024);.

As to the history of 1024, I think it was chosen so to keep some kind of "red zone" so addresses that are zero, or close the zero would end up clobbering the red zone, which can then be detected by the runtime, I also believe this value was being used by emscripten prior the existence of wasm-ld.

I do think it seems reasonable though. I guess we would argue it could be smaller? Maybe 128 or 512? But this change is not about changing that value.

[sbc100]

• add comment

[sunfish]

Looks good.

lld/test/wasm/global-base.test

 RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %t.o
 
+# Check for error on `--global-base` with `-shared` and `-pie`
+RUN: not wasm-ld --global-base=2048 --experimental-pic -shared -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=SHARED-ERROR
+RUN: not wasm-ld --global-base=2048 --experimental-pic -pie -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=SHARED-ERROR
+SHARED-ERROR: error: --global-base may not be used with -shared/-pie
+
+# Check for error on `--global-base` which is lower than that end of the stack
+# when `--stack-first` is used.
+RUN: not wasm-ld --global-base=2048 --stack-first -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=STACK-ERROR
+STACK-ERROR: error: --global-base cannot be less than stack size when --stack-first is used
+
 RUN: wasm-ld --export=__global_base --export=__data_end --allow-undefined -o %t.wasm %t.o
 RUN: obj2yaml %t.wasm | FileCheck %s  -check-prefix=CHECK-1024
 CHECK-1024:       - Type:            GLOBAL
 CHECK-1024-NEXT:    Globals:
 CHECK-1024-NEXT:      - Index:           0
 CHECK-1024-NEXT:        Type:            I32
 CHECK-1024-NEXT:        Mutable:         true
 CHECK-1024-NEXT:        InitExpr:

lld/wasm/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->thinLTOCachePolicy = CHECK(
       parseCachePruningPolicy(args.getLastArgValue(OPT_thinlto_cache_policy)),
       "--thinlto-cache-policy: invalid cache policy");
   config->unresolvedSymbols = getUnresolvedSymbolPolicy(args);
   errorHandler().verbose = args.hasArg(OPT_verbose);
   LLVM_DEBUG(errorHandler().verbose = true);
 
   config->initialMemory = args::getInteger(args, OPT_initial_memory, 0);
-  config->globalBase = args::getInteger(args, OPT_global_base, 1024);
+  config->globalBase = args::getInteger(args, OPT_global_base, 0);
   config->maxMemory = args::getInteger(args, OPT_max_memory, 0);
   config->zStackSize =
       args::getZOptionValue(args, OPT_z, "stack-size", WasmPageSize);
 
   // Default value of exportDynamic depends on `-shared`
   config->exportDynamic =
       args.hasFlag(OPT_export_dynamic, OPT_no_export_dynamic, config->shared);
 
if (config->relocatable) {
     if (config->compressRelocations)
       error("-r -and --compress-relocations may not be used together");
     if (args.hasArg(OPT_undefined))
       error("-r -and --undefined may not be used together");
     if (config->pie)
       error("-r and -pie may not be used together");
     if (config->sharedMemory)
       error("-r and --shared-memory may not be used together");
+    if (config->globalBase)
+      error("-r and --global-base may not by used together");
   }
 
   // To begin to prepare for Module Linking-style shared libraries, start
   // warning about uses of `-shared` and related flags outside of Experimental
   // mode, to give anyone using them a heads-up that they will be changing.
   //
   // Also, warn about flags which request explicit exports.
   if (!config->experimentalPic) {
if (config->unresolvedSymbols == UnresolvedPolicy::ImportDynamic) {
       warn("dynamic imports are not yet stable "
            "(--unresolved-symbols=import-dynamic)");
     }
   }
 
   if (config->bsymbolic && !config->shared) {
     warn("-Bsymbolic is only meaningful when combined with -shared");
   }
+
+  if (config->globalBase && config->isPic) {
+    error("--global-base may not be used with -shared/-pie");
+  }
 }
 
 // Force Sym to be entered in the output. Used for -u or equivalent.
 static Symbol *handleUndefined(StringRef name) {
   Symbol *sym = symtab->find(name);
   if (!sym)
     return nullptr;
 

lld/wasm/Writer.cpp

static void setGlobalPtr(DefinedGlobal *g, uint64_t memoryPtr) {
   LLVM_DEBUG(dbgs() << "setGlobalPtr " << g->getName() << " -> " << memoryPtr << "\n");
   g->global->setPointerValue(memoryPtr);
 }
 
 // Fix the memory layout of the output binary.  This assigns memory offsets
 // to each of the input data sections as well as the explicit stack region.
 // The default memory layout is as follows, from low to high.
 //
-//  - initialized data (starting at Config->globalBase)
+//  - initialized data (starting at config->globalBase)
 //  - BSS data (not currently implemented in llvm)
-//  - explicit stack (Config->ZStackSize)
+//  - explicit stack (config->ZStackSize)
 //  - heap start / unallocated
 //
 // The --stack-first option means that stack is placed before any static data.
 // This can be useful since it means that stack overflow traps immediately
 // rather than overwriting global data, but also increases code size since all
 // static data loads and stores requires larger offsets.
 void Writer::layoutMemory() {
   uint64_t memoryPtr = 0;
auto placeStack = [&]() {
     setGlobalPtr(cast<DefinedGlobal>(WasmSym::stackPointer), memoryPtr);
     if (WasmSym::stackHigh)
       WasmSym::stackHigh->setVA(memoryPtr);
     log("mem: stack top   = " + Twine(memoryPtr));
   };
 
   if (config->stackFirst) {
     placeStack();
+    if (config->globalBase) {
+      if (config->globalBase < memoryPtr) {
+        error("--global-base cannot be less than stack size when --stack-first is used");
+        return;
+      }
+      memoryPtr = config->globalBase;
+    }
   } else {
+    if (!config->globalBase && !config->relocatable && !config->isPic) {
+      // The default offset for static/global data, for when --global-base is
+      // not specified on the command line.  The precise value of 1024 is
+      // somewhat arbitrary, and pre-dates wasm-ld (Its the value that
+      // emscripten used prior to wasm-ld).
+      config->globalBase = 1024;
+    }
     memoryPtr = config->globalBase;
-    log("mem: global base = " + Twine(config->globalBase));
   }
 
+  log("mem: global base = " + Twine(memoryPtr));
   if (WasmSym::globalBase)
     WasmSym::globalBase->setVA(memoryPtr);
 
   uint64_t dataStart = memoryPtr;
 
   // Arbitrarily set __dso_handle handle to point to the start of the data
   // segments.
   if (WasmSym::dsoHandle)

 
 void Writer::createSyntheticSectionsPostLayout() {
   out.dataCountSec = make<DataCountSection>(segments);
   out.linkingSec = make<LinkingSection>(initFunctions, segments);
   out.nameSec = make<NameSection>(segments);
 }
 
 void Writer::run() {
-  if (config->relocatable || config->isPic)
-    config->globalBase = 0;
-
   // For PIC code the table base is assigned dynamically by the loader.
   // For non-PIC, we start at 1 so that accessing table index 0 always traps.
   if (!config->isPic) {
     config->tableBase = 1;
     if (WasmSym::definedTableBase)
       WasmSym::definedTableBase->setVA(config->tableBase);
     if (WasmSym::definedTableBase32)
       WasmSym::definedTableBase32->setVA(config->tableBase);