[PATCH] D44393: [WebAssembly] Add missing implementation for --initial/max-memory args

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/trunk/wasm/Writer.cpp:615
+      error("initial memory too small, " + Twine(MemoryPtr) + " bytes needed");
+    else
+      MemoryPtr = Config->InitialMemory;
----------------
ncw wrote:
> ruiu wrote:
> > This `else` looks odd because if there is an error in the first `if`, the control reaches here, but if there is an error in the second `if`, it doesn't. Maybe we should just set it unconditionally. We won't use a value if there's an error because we'll exit before we use a dummy value.
> You say we'll exit before we use a dummy value - but I don't think exit is no-return?
> 
> In particular, if exit returns then we'll carry on to the MaxMemory block below, which uses MemoryPtr, so we have to leave this block with a "safe" value for MemoryPtr.
> 
> That was my logic anyway.
I mean error() counts the number of errors we've seen and before calling write(), we first check if the counter is zero, and if not, we return without calling that function (which will result in existing from the program.) So we do not really care too much about a wrong value in Config.

And with this code, a wrong value could still be set to MemoryPtr. If the first error test passes but the second test succeeds, this line is executed. That's inconsistent.


Repository:
  rL LLVM

https://reviews.llvm.org/D44393