[WebAssembly] Use StringSaver to retain ownership of ctor function body. NFC

[ncw]

Summary

Notes:

This makes it easier to create other synthetic functions, which don't need the SyntheticFunction instance to own the body as a std::string.

I should have done this as part of D43933, but at the time I thought what I was doing was better... oh well.

[sbc100]

wasm/InputChunks.h

159 ↗

(On Diff #137185)

Why add getComdat() here? If we really want this to be NFC maybe this part should be separate.

[ruiu]

wasm/Writer.cpp

877 ↗

(On Diff #137185)

I don't think the amount of data we need to copy is large, so it is presumably OK, and I'm not obsessed with micro-optimization, but here we make a copy of std::string here and then immediately discard the original string at the end of the scope. So this makes a redundant copy of a string.

There is a way to avoid unnecessary copying. If you allocate std::string by make<std::string>(), then its body will have the same lifetime as the entire linking process.

[ncw]

wasm/InputChunks.h

159 ↗

(On Diff #137185)

Oh ah - that slipped in. It is NFC because getComdat() currently isn't called (it actually crashes currently), so it can't be changing any existing behaviour that doesn't crash. That chunk just got pulled in "under the radar" because it's next the lines I picked into this PR, sorry. It's necessary for the synthetic undefined stubs.

I could put it in another commit, but it kind of fits in with the "fixes to SyntheticFunction" theme.

wasm/Writer.cpp

877 ↗

(On Diff #137185)

True, we could save on that copy. But if we were micro-optimising, make<std::string> would be a bit wasteful too, since make<> would have to create a whole new slab/arena for the std::string (and register the arena so the string's dtor is called).

Best of all we'd maybe do BAlloc.Allocate<char>(5 + 5 + InitFunctions.size()*6 + 1) and then write directly into the interned buffer right away, with no copies at all.

But I agree it's not worth trying to optimise; this commit is just trying to make the SyntheticFunction ctor more generally useful.

Thanks for the review!

lld/trunk/wasm/InputChunks.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_WASM_INPUT_CHUNKS_H
 #define LLD_WASM_INPUT_CHUNKS_H
 
 #include "Config.h"
 #include "InputFiles.h"
 #include "lld/Common/ErrorHandler.h"
-#include "lld/Common/Strings.h"
 #include "llvm/Object/Wasm.h"
 
 using llvm::object::WasmSegment;
 using llvm::wasm::WasmFunction;
 using llvm::wasm::WasmRelocation;
 using llvm::wasm::WasmSignature;
 using llvm::object::WasmSection;
 
protected:
 
   const WasmFunction *Function;
   llvm::Optional<uint32_t> OutputIndex;
   llvm::Optional<uint32_t> TableIndex;
 };
 
 class SyntheticFunction : public InputFunction {
 public:
-  SyntheticFunction(const WasmSignature &S, std::string Body, StringRef Name)
-      : InputFunction(S, nullptr, nullptr), Name(Name), Body(std::move(Body)) {}
+  SyntheticFunction(const WasmSignature &S, ArrayRef<uint8_t> Body,
+                    StringRef Name)
+      : InputFunction(S, nullptr, nullptr), Name(Name), Body(Body) {}
 
   StringRef getName() const override { return Name; }
+  StringRef getComdat() const override { return StringRef(); }
 
 protected:
-  ArrayRef<uint8_t> data() const override { return toArrayRef(Body); }
+  ArrayRef<uint8_t> data() const override { return Body; }
 
   StringRef Name;
-  std::string Body;
+  ArrayRef<uint8_t> Body;
 };
 
 } // namespace wasm
 
 std::string toString(const wasm::InputChunk *);
 } // namespace lld
 
 #endif // LLD_WASM_INPUT_CHUNKS_H

lld/trunk/wasm/Writer.cpp

 #include "InputChunks.h"
 #include "InputGlobal.h"
 #include "OutputSections.h"
 #include "OutputSegment.h"
 #include "SymbolTable.h"
 #include "WriterUtils.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
+#include "lld/Common/Strings.h"
 #include "lld/Common/Threads.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/LEB128.h"
 
void Writer::createCtorFunction() {
   {
     raw_string_ostream OS(FunctionBody);
     writeUleb128(OS, BodyContent.size(), "function size");
     OS << BodyContent;
   }
 
   const WasmSignature *Sig = WasmSym::CallCtors->getFunctionType();
   SyntheticFunction *F = make<SyntheticFunction>(
-      *Sig, std::move(FunctionBody), WasmSym::CallCtors->getName());
+      *Sig, toArrayRef(Saver.save(FunctionBody)),
+      WasmSym::CallCtors->getName());
 
   F->setOutputIndex(FunctionIndex);
   F->Live = true;
   WasmSym::CallCtors->Function = F;
   InputFunctions.emplace_back(F);
 }
 
 // Populate InitFunctions vector with init functions from all input objects.