[PATCH] D40559: Run Wasm entrypoint on load

[Nicholas Wilson via Phabricator via llvm-commits]

ncw created this revision.
Herald added subscribers: llvm-commits, sunfish, aheejin, dschuff, jfb.

This was originally posted on GitHub here: https://github.com/WebAssembly/lld/pull/15

Now that the GitHub repo has been archived, I can't post a reply there! So I'm opening this for discussion despite Sam's lack of enthusiasm...

I said:

> The --entry option is there to allow designating the entry-point symbol, but it isn't wired in!
> 
> LLD needs to actually set the entrypoint on the Wasm output, using the "start" section.

Sam said:

> The reason we do this right now is for parity the expectations of emscripten and also wasm.js in the musl repo (which we use for testing):
>  https://github.com/jfbastien/musl/blob/wasm-prototype-1/arch/wasm32/wasm.js
> 
> Both these loaders expect to be able to run the main function explicitly and get the return code from it.
> 
> There has been some discussion about the role of the start section and how the linker should use it. I think it was originally intented to tune the C/C++ internalizer functions but not run main(). This is certainly one concievable option, but it would involve the linker generating said function, or assuming its name (in musl this i called __libc_start_init for example).
> 
> For now I think it makes sense to keep it empty and let the embedder choose to run what it wants when it whats rather than on instantiate.

My response:

I see where you're coming from... but surely the whole point of the "entrypoint" argument is to determine which symbol (if any) should be set as the Wasm module's entry point.

If in fact you'd like it so that the default is not to have an entrypoint, then we could easily change the default to be `--entry ''` (empty string). But for those of us who do want an entrypoint on our Wasm modules, it would be possible to do that.

I agree that we don't really want "main" to be run as part of the Wasm entrypoint.

Maybe the long-term solution is to fiddle with Musl, to create a new function "_start_wasm" that calls both `__init_libc` and `__libc_start_init` (but not main), and make that function name part of the Wasm "linkage conventions".

That would be basically this PR, plus a one-liner to change "_start" to "_start_wasm".

Finally, the reason why I'm interesting in actually using Wasm's "start" functionality is because it seems to me to be a better form level of encapsulation. No-one really wants it to be possible to touch global variables that aren't initialised, I think it is preferable that the WebAssembly instantiation fails if the globals can't be initialised, and make it impossible to invoke exports on an uninitialised module.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40559

Files:
  wasm/Driver.cpp
  wasm/Writer.cpp


Index: wasm/Writer.cpp
===================================================================
--- wasm/Writer.cpp
+++ wasm/Writer.cpp
@@ -305,22 +305,6 @@
     writeExport(OS, MemoryExport);
   }
 
-  if (ExportMain) {
-    Symbol *Sym = Symtab->find(Config->Entry);
-    if (Sym->isDefined()) {
-      if (!Sym->isFunction())
-        fatal("entry point is not a function: " + Sym->getName());
-
-      if (!ExportOther) {
-        WasmExport MainExport;
-        MainExport.Name = Config->Entry;
-        MainExport.Kind = WASM_EXTERNAL_FUNCTION;
-        MainExport.Index = Sym->getOutputIndex();
-        writeExport(OS, MainExport);
-      }
-    }
-  }
-
   if (ExportOther) {
     for (ObjFile *File : Symtab->ObjectFiles) {
       for (Symbol *Sym : File->getSymbols()) {
@@ -345,7 +329,22 @@
   }
 }
 
-void Writer::createStartSection() {}
+void Writer::createStartSection() {
+  if (Config->Entry.empty())
+    return;
+
+  const Symbol *Sym = Symtab->find(Config->Entry);
+  if (!Sym)
+    fatal("entry point not found: " + Config->Entry);
+  if (!Sym->isFunction())
+    fatal("entry point is not a function: " + Sym->getName());
+
+  SyntheticSection *Section = createSyntheticSection(WASM_SEC_START);
+  raw_ostream &OS = Section->getStream();
+
+  log("Start: " + Sym->getName());
+  writeUleb128(OS, Sym->getOutputIndex(), "start index");
+}
 
 void Writer::createElemSection() {
   if (!NumElements)
Index: wasm/Driver.cpp
===================================================================
--- wasm/Driver.cpp
+++ wasm/Driver.cpp
@@ -292,10 +292,11 @@
 
   Config->AllowUndefined = Args.hasArg(OPT_allow_undefined);
   Config->EmitRelocs = Args.hasArg(OPT_emit_relocs);
-  Config->Entry = Args.getLastArgValue(OPT_entry);
+  Config->Relocatable = Args.hasArg(OPT_relocatable);
+  Config->Entry = Args.getLastArgValue(OPT_entry,
+                                       Config->Relocatable ? "" : "_start");
   Config->ImportMemory = Args.hasArg(OPT_import_memory);
   Config->OutputFile = Args.getLastArgValue(OPT_o);
-  Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SearchPaths = getArgs(Args, OPT_L);
   Config->StripAll = Args.hasArg(OPT_strip_all);
   Config->StripDebug = Args.hasArg(OPT_strip_debug);
@@ -323,9 +324,8 @@
     error("entry point specified for relocatable output file");
 
   if (!Config->Relocatable) {
-    if (Config->Entry.empty())
-      Config->Entry = "_start";
-    addSyntheticUndefinedFunction(Config->Entry);
+    if (!Config->Entry.empty())
+      addSyntheticUndefinedFunction(Config->Entry);
 
     addSyntheticGlobal("__stack_pointer", 0);
   }
@@ -348,6 +348,8 @@
 
   if (!Config->Entry.empty()) {
     Symbol *Sym = Symtab->find(Config->Entry);
+    if (!Sym)
+      fatal("entry point not found: " + Config->Entry);
     if (!Sym->isFunction())
       fatal("entry point is not a function: " + Sym->getName());
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40559.124569.patch
Type: text/x-patch
Size: 2887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171128/08698947/attachment.bin>