[wasm] Improve pimpl implementation in WasmInterpreter::Thread

src/wasm/wasm-interpreter.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/wasm/wasm-interpreter.h"
 
 #include "src/utils.h"
 #include "src/wasm/decoder.h"
 #include "src/wasm/function-body-decoder.h"
 #include "src/wasm/wasm-external-refs.h"
@@ skipping 900 matching lines @@
     code->targets = nullptr;
     code->orig_start = start;
     code->orig_end = end;
     code->start = const_cast<byte*>(start);
     code->end = const_cast<byte*>(end);
     Preprocess(code);
     return true;
   }
 };
 
+namespace {
 // Responsible for executing code directly.
-class WasmInterpreter::ThreadImpl : public ZoneObject {
+class ThreadImpl {
  public:
   ThreadImpl(Zone* zone, CodeMap* codemap, WasmInstance* instance)
       : codemap_(codemap),
         instance_(instance),
         stack_(zone),
         frames_(zone),
         blocks_(zone),
         state_(WasmInterpreter::STOPPED),
         break_pc_(kInvalidPc),
         trap_reason_(kTrapCount),
         possible_nondeterminism_(false) {}
 
-  ~ThreadImpl() {}
-
   //==========================================================================
   // Implementation of public interface for WasmInterpreter::Thread.
   //==========================================================================
 
   WasmInterpreter::State state() { return state_; }
 
   void PushFrame(const WasmFunction* function, WasmVal* args) {
     InterpreterCode* code = codemap()->FindCode(function);
     CHECK_NOT_NULL(code);
     frames_.push_back({code, 0, 0, stack_.size()});
@@ skipping 744 matching lines @@
             break;
           default:
             UNREACHABLE();
             break;
         }
       }
     }
   }
 };
 
+// Converters between WasmInterpreter::Thread and WasmInterpreter::ThreadImpl.
+// Thread* is the public interface, without knowledge of the object layout.
+// This cast is potentially risky, but as long as we always cast it back before
+// accessing any data, it should be fine. UBSan is not complaining.
+WasmInterpreter::Thread* ToThread(ThreadImpl* impl) {
+  return reinterpret_cast<WasmInterpreter::Thread*>(impl);
+}
+static ThreadImpl* ToImpl(WasmInterpreter::Thread* thread) {
+  return reinterpret_cast<ThreadImpl*>(thread);
+}
+}  // namespace
+
 //============================================================================
-// Forwarding functions of WasmInterpreter::Thread (pimpl idiom).
+// Implementation of the pimpl idiom for WasmInterpreter::Thread.
+// Instead of placing a pointer to the ThreadImpl inside of the Thread object,
+// we just reinterpret_cast them. ThreadImpls are only allocated inside this
+// translation unit anyway.
 //============================================================================
-WasmInterpreter::Thread::Thread(ThreadImpl* impl) : impl_(impl) {}
 WasmInterpreter::State WasmInterpreter::Thread::state() {
-  return impl_->state();
+  return ToImpl(this)->state();
 }
 void WasmInterpreter::Thread::PushFrame(const WasmFunction* function,
                                         WasmVal* args) {
-  return impl_->PushFrame(function, args);
+  return ToImpl(this)->PushFrame(function, args);
 }
-WasmInterpreter::State WasmInterpreter::Thread::Run() { return impl_->Run(); }
-WasmInterpreter::State WasmInterpreter::Thread::Step() { return impl_->Step(); }
-void WasmInterpreter::Thread::Pause() { return impl_->Pause(); }
-void WasmInterpreter::Thread::Reset() { return impl_->Reset(); }
+WasmInterpreter::State WasmInterpreter::Thread::Run() {
+  return ToImpl(this)->Run();
+}
+WasmInterpreter::State WasmInterpreter::Thread::Step() {
+  return ToImpl(this)->Step();
+}
+void WasmInterpreter::Thread::Pause() { return ToImpl(this)->Pause(); }
+void WasmInterpreter::Thread::Reset() { return ToImpl(this)->Reset(); }
 pc_t WasmInterpreter::Thread::GetBreakpointPc() {
-  return impl_->GetBreakpointPc();
+  return ToImpl(this)->GetBreakpointPc();
 }
-int WasmInterpreter::Thread::GetFrameCount() { return impl_->GetFrameCount(); }
+int WasmInterpreter::Thread::GetFrameCount() {
+  return ToImpl(this)->GetFrameCount();
+}
 const WasmFrame* WasmInterpreter::Thread::GetFrame(int index) {
-  return impl_->GetFrame(index);
+  return ToImpl(this)->GetFrame(index);
 }
 WasmFrame* WasmInterpreter::Thread::GetMutableFrame(int index) {
-  return impl_->GetMutableFrame(index);
+  return ToImpl(this)->GetMutableFrame(index);
 }
 WasmVal WasmInterpreter::Thread::GetReturnValue(int index) {
-  return impl_->GetReturnValue(index);
+  return ToImpl(this)->GetReturnValue(index);
 }
 bool WasmInterpreter::Thread::PossibleNondeterminism() {
-  return impl_->PossibleNondeterminism();
+  return ToImpl(this)->PossibleNondeterminism();
 }
 
 //============================================================================
 // The implementation details of the interpreter.
 //============================================================================
 class WasmInterpreterInternals : public ZoneObject {
  public:
   WasmInstance* instance_;
   // Create a copy of the module bytes for the interpreter, since the passed
   // pointer might be invalidated after constructing the interpreter.
   const ZoneVector<uint8_t> module_bytes_;
   CodeMap codemap_;
-  ZoneVector<WasmInterpreter::Thread> threads_;
+  ZoneVector<ThreadImpl> threads_;
 
   WasmInterpreterInternals(Zone* zone, const ModuleBytesEnv& env)
       : instance_(env.instance),
         module_bytes_(env.module_bytes.start(), env.module_bytes.end(), zone),
         codemap_(env.instance ? env.instance->module : nullptr,
                  module_bytes_.data(), zone),
         threads_(zone) {
-    threads_.push_back(WasmInterpreter::Thread(
-        new (zone) WasmInterpreter::ThreadImpl(zone, &codemap_, env.instance)));
+    threads_.push_back(ThreadImpl(zone, &codemap_, env.instance));
   }
 
   void Delete() { threads_.clear(); }
 };
 
 //============================================================================
 // Implementation of the public interface of the interpreter.
 //============================================================================
 WasmInterpreter::WasmInterpreter(const ModuleBytesEnv& env,
                                  AccountingAllocator* allocator)
@@ skipping 42 matching lines @@
   UNIMPLEMENTED();
   return false;
 }
 
 int WasmInterpreter::GetThreadCount() {
   return 1;  // only one thread for now.
 }
 
 WasmInterpreter::Thread* WasmInterpreter::GetThread(int id) {
   CHECK_EQ(0, id);  // only one thread for now.
-  return &internals_->threads_[id];
+  return ToThread(&internals_->threads_[id]);
 }
 
 WasmVal WasmInterpreter::GetLocalVal(const WasmFrame* frame, int index) {
   CHECK_GE(index, 0);
   UNIMPLEMENTED();
   WasmVal none;
   none.type = kWasmStmt;
   return none;
 }
 
@@ skipping 37 matching lines @@
 
 ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting(
     Zone* zone, const byte* start, const byte* end) {
   ControlTransfers targets(zone, nullptr, start, end);
   return targets.map_;
 }
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8