[PATCH] D59343: [WebAssembly] Use passive segments when memory is shared

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 16 10:50:39 PDT 2019


sbc100 added inline comments.


================
Comment at: lld/wasm/Driver.cpp:447
       // For PIC code we create a synthetic function call __wasm_apply_relocs
-      // and add this as the first call in __wasm_call_ctors.
-      // We also unconditionally export 
+      // and add this as the second call in __wasm_call_ctors.
+      // We also unconditionally export
----------------
It might not be the second so perhaps how about:  "which is called from __wasm_call_ctors before the user-level constructors".


================
Comment at: lld/wasm/MarkLive.cpp:81
     Enqueue(WasmSym::CallCtors);
+  }
+
----------------
The coding style for lld is to not include braces for single line blocks.

We need to think a little more about this is going to work for non-PIC code.  Up until now its been possible to write non-PIC static executable that runs the ctors itself via some arbitrary function. e.g.

```
int mystartfunc() {
  __wasm_call_ctors();
  return main()
}
```

This part of the markLive is designed to keep `__wasm_call_ctors` alive even if the user never references it (since its to be exported in the PIC case).

So.. in the non-PIC case, even for shared-memory, I think we should let the normal GC process take its course.   If they user forgets to reference __wasm_call_ctors, then there is not way it can ever be called, so we might as well let it be GC'd.

But the dependency setup here is getting a little complicated.  Really what we want to say is something like:

"if CallCtors is alive then ApplyRelocs need to be too" and the same for InitMemory.  Perhaps we should add `Dependencies()` to SyntheticFunction?

For now I'm ok with the conservative approach of just keeping InitMemory alive as long as SharedMemory is present.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59343/new/

https://reviews.llvm.org/D59343





More information about the llvm-commits mailing list