Sanitize MCJIT remote execution

MCJIT remote execution (ChildTarget+RemoteTargetExternal) protocol was in
dire need of refactoring. It was fail-prone, had no error reporting and
implemented the same message logic on every single function.

This patch rectifies it, and makes it work on ARM, where it was randomly
failing. Other architectures shall profit from this change as well, making
their buildbots and releases more reliable.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@199261 91177308-0d34-0410-b5e6-96231b3b80d8

tools/lli/ChildTarget/ChildTarget.cpp

@@ -25,8 +25,8 @@ private:
   // Outgoing message handlers
   void sendChildActive();
   void sendAllocationResult(uint64_t Addr);
-  void sendLoadComplete();
+  void sendLoadStatus(uint32_t Status);
-  void sendExecutionComplete(uint64_t Result);
+  void sendExecutionComplete(int Result);
 
   // OS-specific functions
   void initializeConnection();
@@ -36,6 +36,7 @@ private:
   void makeSectionExecutable(uint64_t Addr, uint32_t Size);
   void InvalidateInstructionCache(const void *Addr, size_t Len);
   void releaseMemory(uint64_t Addr, uint32_t Size);
+  bool isAllocatedMemory(uint64_t Address, uint32_t Size);
 
   // Store a map of allocated buffers to sizes.
   typedef std::map<uint64_t, uint32_t> AllocMapType;
@@ -121,33 +122,36 @@ void LLIChildTarget::handleLoadSection(bool IsCode) {
   // Read the message data size.
   uint32_t DataSize;
   int rc = ReadBytes(&DataSize, 4);
+  (void)rc;
   assert(rc == 4);
 
   // Read the target load address.
   uint64_t Addr;
   rc = ReadBytes(&Addr, 8);
   assert(rc == 8);
-
   size_t BufferSize = DataSize - 8;
 
-  // FIXME: Verify that this is in allocated space
+  if (!isAllocatedMemory(Addr, BufferSize))
+    return sendLoadStatus(LLI_Status_NotAllocated);
 
   // Read section data into previously allocated buffer
-  rc = ReadBytes((void*)Addr, DataSize - 8);
+  rc = ReadBytes((void*)Addr, BufferSize);
-  assert(rc == (int)(BufferSize));
+  if (rc != (int)(BufferSize))
+    return sendLoadStatus(LLI_Status_IncompleteMsg);
 
   // If IsCode, mark memory executable
   if (IsCode)
     makeSectionExecutable(Addr, BufferSize);
 
   // Send MarkLoadComplete message.
-  sendLoadComplete();
+  sendLoadStatus(LLI_Status_Success);
 }
 
 void LLIChildTarget::handleExecute() {
   // Read the message data size.
   uint32_t DataSize;
   int rc = ReadBytes(&DataSize, 4);
+  (void)rc;
   assert(rc == 4);
   assert(DataSize == 8);
 
@@ -162,7 +166,7 @@ void LLIChildTarget::handleExecute() {
   Result = fn();
 
   // Send ExecutionResult message.
-  sendExecutionComplete((int64_t)Result);
+  sendExecutionComplete(Result);
 }
 
 void LLIChildTarget::handleTerminate() {
@@ -175,11 +179,26 @@ void LLIChildTarget::handleTerminate() {
   m_AllocatedBufferMap.clear();
 }
 
+bool LLIChildTarget::isAllocatedMemory(uint64_t Address, uint32_t Size) {
+  uint64_t End = Address+Size;
+  AllocMapType::iterator ItBegin = m_AllocatedBufferMap.begin();
+  AllocMapType::iterator ItEnd = m_AllocatedBufferMap.end();
+  for (AllocMapType::iterator It = ItBegin; It != ItEnd; ++It) {
+    uint64_t A = It->first;
+    uint64_t E = A + It->second;
+    // Starts and finishes inside allocated region
+    if (Address >= A && End <= E)
+      return true;
+  }
+  return false;
+}
+
 // Outgoing message handlers
 void LLIChildTarget::sendChildActive() {
   // Write the message type.
   uint32_t MsgType = (uint32_t)LLI_ChildActive;
   int rc = WriteBytes(&MsgType, 4);
+  (void)rc;
   assert(rc == 4);
 
   // Write the data size.
@@ -192,6 +211,7 @@ void LLIChildTarget::sendAllocationResult(uint64_t Addr) {
   // Write the message type.
   uint32_t MsgType = (uint32_t)LLI_AllocationResult;
   int rc = WriteBytes(&MsgType, 4);
+  (void)rc;
   assert(rc == 4);
 
   // Write the data size.
@@ -204,33 +224,39 @@ void LLIChildTarget::sendAllocationResult(uint64_t Addr) {
   assert(rc == 8);
 }
 
-void LLIChildTarget::sendLoadComplete() {
+void LLIChildTarget::sendLoadStatus(uint32_t Status) {
   // Write the message type.
-  uint32_t MsgType = (uint32_t)LLI_LoadComplete;
+  uint32_t MsgType = (uint32_t)LLI_LoadResult;
   int rc = WriteBytes(&MsgType, 4);
+  (void)rc;
   assert(rc == 4);
 
   // Write the data size.
-  uint32_t DataSize = 0;
+  uint32_t DataSize = 4;
-  rc = WriteBytes(&DataSize, 4);
-  assert(rc == 4);
-}
-
-void LLIChildTarget::sendExecutionComplete(uint64_t Result) {
-  // Write the message type.
-  uint32_t MsgType = (uint32_t)LLI_ExecutionResult;
-  int rc = WriteBytes(&MsgType, 4);
-  assert(rc == 4);
-
-
-  // Write the data size.
-  uint32_t DataSize = 8;
   rc = WriteBytes(&DataSize, 4);
   assert(rc == 4);
 
   // Write the result.
-  rc = WriteBytes(&Result, 8);
+  rc = WriteBytes(&Status, 4);
-  assert(rc == 8);
+  assert(rc == 4);
+}
+
+void LLIChildTarget::sendExecutionComplete(int Result) {
+  // Write the message type.
+  uint32_t MsgType = (uint32_t)LLI_ExecutionResult;
+  int rc = WriteBytes(&MsgType, 4);
+  (void)rc;
+  assert(rc == 4);
+
+
+  // Write the data size.
+  uint32_t DataSize = 4;
+  rc = WriteBytes(&DataSize, 4);
+  assert(rc == 4);
+
+  // Write the result.
+  rc = WriteBytes(&Result, 4);
+  assert(rc == 4);
 }
 
 #ifdef LLVM_ON_UNIX

tools/lli/RemoteMemoryManager.cpp

@@ -129,7 +129,7 @@ void RemoteMemoryManager::notifyObjectLoaded(ExecutionEngine *EE,
 
   // Allocate space in the remote target.
   uint64_t RemoteAddr;
-  if (Target->allocateSpace(CurOffset, MaxAlign, RemoteAddr))
+  if (!Target->allocateSpace(CurOffset, MaxAlign, RemoteAddr))
     report_fatal_error(Target->getErrorMsg());
 
   // Map the section addresses so relocations will get updated in the local
@@ -155,13 +155,13 @@ bool RemoteMemoryManager::finalizeMemory(std::string *ErrMsg) {
     uint64_t RemoteAddr = I->first;
     const Allocation &Section = I->second;
     if (Section.IsCode) {
-      Target->loadCode(RemoteAddr, Section.MB.base(), Section.MB.size());
+      if (!Target->loadCode(RemoteAddr, Section.MB.base(), Section.MB.size()))
-
+        report_fatal_error(Target->getErrorMsg());
       DEBUG(dbgs() << "  loading code: " << Section.MB.base()
             << " to remote: 0x" << format("%llx", RemoteAddr) << "\n");
     } else {
-      Target->loadData(RemoteAddr, Section.MB.base(), Section.MB.size());
+      if (!Target->loadData(RemoteAddr, Section.MB.base(), Section.MB.size()))
-
+        report_fatal_error(Target->getErrorMsg());
       DEBUG(dbgs() << "  loading data: " << Section.MB.base()
             << " to remote: 0x" << format("%llx", RemoteAddr) << "\n");
     }

tools/lli/RemoteTarget.cpp

@@ -56,34 +56,35 @@ bool RemoteTarget::allocateSpace(size_t Size, unsigned Alignment,
   sys::MemoryBlock *Prev = Allocations.size() ? &Allocations.back() : NULL;
   sys::MemoryBlock Mem = sys::Memory::AllocateRWX(Size, Prev, &ErrorMsg);
   if (Mem.base() == NULL)
-    return true;
+    return false;
   if ((uintptr_t)Mem.base() % Alignment) {
     ErrorMsg = "unable to allocate sufficiently aligned memory";
-    return true;
+    return false;
   }
   Address = reinterpret_cast<uint64_t>(Mem.base());
-  return false;
+  return true;
 }
 
 bool RemoteTarget::loadData(uint64_t Address, const void *Data, size_t Size) {
   memcpy ((void*)Address, Data, Size);
-  return false;
+  return true;
 }
 
 bool RemoteTarget::loadCode(uint64_t Address, const void *Data, size_t Size) {
   memcpy ((void*)Address, Data, Size);
   sys::MemoryBlock Mem((void*)Address, Size);
   sys::Memory::setExecutable(Mem, &ErrorMsg);
-  return false;
+  return true;
 }
 
 bool RemoteTarget::executeCode(uint64_t Address, int &RetVal) {
   int (*fn)(void) = (int(*)(void))Address;
   RetVal = fn();
-  return false;
+  return true;
 }
 
-void RemoteTarget::create() {
+bool RemoteTarget::create() {
+  return true;
 }
 
 void RemoteTarget::stop() {

tools/lli/RemoteTarget.h

@@ -25,11 +25,13 @@
 namespace llvm {
 
 class RemoteTarget {
-  std::string ErrorMsg;
   bool IsRunning;
 
   SmallVector<sys::MemoryBlock, 16> Allocations;
 
+protected:
+  std::string ErrorMsg;
+
 public:
   StringRef getErrorMsg() const { return ErrorMsg; }
 
@@ -39,7 +41,7 @@ public:
   /// @param      Alignment Required minimum alignment for allocated space.
   /// @param[out] Address   Remote address of the allocated memory.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool allocateSpace(size_t Size,
                              unsigned Alignment,
@@ -51,7 +53,7 @@ public:
   /// @param      Data      Source address in the host process.
   /// @param      Size      Number of bytes to copy.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool loadData(uint64_t Address,
                         const void *Data,
@@ -63,7 +65,7 @@ public:
   /// @param      Data      Source address in the host process.
   /// @param      Size      Number of bytes to copy.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool loadCode(uint64_t Address,
                         const void *Data,
@@ -76,7 +78,7 @@ public:
   ///                       process.
   /// @param[out] RetVal    The integer return value of the called function.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool executeCode(uint64_t Address,
                            int &RetVal);
@@ -89,12 +91,12 @@ public:
   virtual unsigned getPageAlignment() { return 4096; }
 
   /// Start the remote process.
-  virtual void create();
+  virtual bool create();
 
   /// Terminate the remote process.
   virtual void stop();
 
-  RemoteTarget() : ErrorMsg(""), IsRunning(false) {}
+  RemoteTarget() : IsRunning(false), ErrorMsg("") {}
   virtual ~RemoteTarget() { if (IsRunning) stop(); }
 
   // Create an instance of the system-specific remote target class.

tools/lli/RemoteTargetExternal.cpp

@@ -17,6 +17,8 @@
 #include "RemoteTargetExternal.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/Memory.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
@@ -26,28 +28,85 @@ using namespace llvm;
 
 bool RemoteTargetExternal::allocateSpace(size_t Size, unsigned Alignment,
                                  uint64_t &Address) {
-  SendAllocateSpace(Alignment, Size);
+  DEBUG(dbgs() << "Message [allocate space] size: " << Size <<
-  Receive(LLI_AllocationResult, Address);
+                  ", align: " << Alignment << "\n");
+  if (!SendAllocateSpace(Alignment, Size)) {
+    ErrorMsg += ", (RemoteTargetExternal::allocateSpace)";
     return false;
+  }
+  if (!Receive(LLI_AllocationResult, Address)) {
+    ErrorMsg += ", (RemoteTargetExternal::allocateSpace)";
+    return false;
+  }
+  if (Address == 0) {
+    ErrorMsg += "failed allocation, (RemoteTargetExternal::allocateSpace)";
+    return false;
+  }
+  DEBUG(dbgs() << "Message [allocate space] addr: 0x" <<
+                  format("%llx", Address) << "\n");
+  return true;
 }
 
 bool RemoteTargetExternal::loadData(uint64_t Address, const void *Data, size_t Size) {
-  SendLoadSection(Address, Data, (uint32_t)Size, false);
+  DEBUG(dbgs() << "Message [load data] addr: 0x" << format("%llx", Address) <<
-  Receive(LLI_LoadComplete);
+                  ", size: " << Size << "\n");
+  if (!SendLoadSection(Address, Data, (uint32_t)Size, false)) {
+    ErrorMsg += ", (RemoteTargetExternal::loadData)";
     return false;
+  }
+  int Status = LLI_Status_Success;
+  if (!Receive(LLI_LoadResult, Status)) {
+    ErrorMsg += ", (RemoteTargetExternal::loadData)";
+    return false;
+  }
+  if (Status == LLI_Status_IncompleteMsg) {
+    ErrorMsg += "incomplete load data, (RemoteTargetExternal::loadData)";
+    return false;
+  }
+  if (Status == LLI_Status_NotAllocated) {
+    ErrorMsg += "data memory not allocated, (RemoteTargetExternal::loadData)";
+    return false;
+  }
+  DEBUG(dbgs() << "Message [load data] complete\n");
+  return true;
 }
 
 bool RemoteTargetExternal::loadCode(uint64_t Address, const void *Data, size_t Size) {
-  SendLoadSection(Address, Data, (uint32_t)Size, true);
+  DEBUG(dbgs() << "Message [load code] addr: 0x" << format("%llx", Address) <<
-  Receive(LLI_LoadComplete);
+                  ", size: " << Size << "\n");
+  if (!SendLoadSection(Address, Data, (uint32_t)Size, true)) {
+    ErrorMsg += ", (RemoteTargetExternal::loadCode)";
     return false;
+  }
+  int Status = LLI_Status_Success;
+  if (!Receive(LLI_LoadResult, Status)) {
+    ErrorMsg += ", (RemoteTargetExternal::loadCode)";
+    return false;
+  }
+  if (Status == LLI_Status_IncompleteMsg) {
+    ErrorMsg += "incomplete load data, (RemoteTargetExternal::loadData)";
+    return false;
+  }
+  if (Status == LLI_Status_NotAllocated) {
+    ErrorMsg += "data memory not allocated, (RemoteTargetExternal::loadData)";
+    return false;
+  }
+  DEBUG(dbgs() << "Message [load code] complete\n");
+  return true;
 }
 
-bool RemoteTargetExternal::executeCode(uint64_t Address, int &RetVal) {
+bool RemoteTargetExternal::executeCode(uint64_t Address, int32_t &RetVal) {
-  SendExecute(Address);
+  DEBUG(dbgs() << "Message [exectue code] addr: " << Address << "\n");
-
+  if (!SendExecute(Address)) {
-  Receive(LLI_ExecutionResult, RetVal);
+    ErrorMsg += ", (RemoteTargetExternal::executeCode)";
     return false;
+  }
+  if (!Receive(LLI_ExecutionResult, RetVal)) {
+    ErrorMsg += ", (RemoteTargetExternal::executeCode)";
+    return false;
+  }
+  DEBUG(dbgs() << "Message [exectue code] return: " << RetVal << "\n");
+  return true;
 }
 
 void RemoteTargetExternal::stop() {
@@ -55,106 +114,206 @@ void RemoteTargetExternal::stop() {
   Wait();
 }
 
-void RemoteTargetExternal::SendAllocateSpace(uint32_t Alignment, uint32_t Size) {
+bool RemoteTargetExternal::SendAllocateSpace(uint32_t Alignment, uint32_t Size) {
-  int rc;
+  if (!SendHeader(LLI_AllocateSpace)) {
-  (void)rc;
+    ErrorMsg += ", (RemoteTargetExternal::SendAllocateSpace)";
-  uint32_t MsgType = (uint32_t)LLI_AllocateSpace;
+    return false;
-  rc = WriteBytes(&MsgType, 4);
+  }
-  assert(rc == 4 && "Error writing message type.");
 
-  uint32_t DataSize = 8;
+  AppendWrite((const void *)&Alignment, 4);
-  rc = WriteBytes(&DataSize, 4);
+  AppendWrite((const void *)&Size, 4);
-  assert(rc == 4 && "Error writing data size.");
 
-  rc = WriteBytes(&Alignment, 4);
+  if (!SendPayload()) {
-  assert(rc == 4 && "Error writing alignment data.");
+    ErrorMsg += ", (RemoteTargetExternal::SendAllocateSpace)";
-
+    return false;
-  rc = WriteBytes(&Size, 4);
+  }
-  assert(rc == 4 && "Error writing size data.");
+  return true;
 }
 
-void RemoteTargetExternal::SendLoadSection(uint64_t Addr,
+bool RemoteTargetExternal::SendLoadSection(uint64_t Addr,
                                        const void *Data,
                                        uint32_t Size,
                                        bool IsCode) {
-  int rc;
+  LLIMessageType MsgType = IsCode ? LLI_LoadCodeSection : LLI_LoadDataSection;
-  (void)rc;
+  if (!SendHeader(MsgType)) {
-  uint32_t MsgType = IsCode ? LLI_LoadCodeSection : LLI_LoadDataSection;
+    ErrorMsg += ", (RemoteTargetExternal::SendLoadSection)";
-  rc = WriteBytes(&MsgType, 4);
+    return false;
-  assert(rc == 4 && "Error writing message type.");
+  }
 
-  uint32_t DataSize = Size + 8;
+  AppendWrite((const void *)&Addr, 8);
-  rc = WriteBytes(&DataSize, 4);
+  AppendWrite(Data, Size);
-  assert(rc == 4 && "Error writing data size.");
 
-  rc = WriteBytes(&Addr, 8);
+  if (!SendPayload()) {
-  assert(rc == 8 && "Error writing data.");
+    ErrorMsg += ", (RemoteTargetExternal::SendLoadSection)";
-
+    return false;
-  rc = WriteBytes(Data, Size);
+  }
-  assert(rc == (int)Size && "Error writing data.");
+  return true;
 }
 
-void RemoteTargetExternal::SendExecute(uint64_t Addr) {
+bool RemoteTargetExternal::SendExecute(uint64_t Addr) {
-  int rc;
+  if (!SendHeader(LLI_Execute)) {
-  (void)rc;
+    ErrorMsg += ", (RemoteTargetExternal::SendExecute)";
-  uint32_t MsgType = (uint32_t)LLI_Execute;
+    return false;
-  rc = WriteBytes(&MsgType, 4);
+  }
-  assert(rc == 4 && "Error writing message type.");
 
-  uint32_t DataSize = 8;
+  AppendWrite((const void *)&Addr, 8);
-  rc = WriteBytes(&DataSize, 4);
-  assert(rc == 4 && "Error writing data size.");
 
-  rc = WriteBytes(&Addr, 8);
+  if (!SendPayload()) {
-  assert(rc == 8 && "Error writing data.");
+    ErrorMsg += ", (RemoteTargetExternal::SendExecute)";
+    return false;
+  }
+  return true;
 }
 
-void RemoteTargetExternal::SendTerminate() {
+bool RemoteTargetExternal::SendTerminate() {
-  int rc;
+  return SendHeader(LLI_Terminate);
-  (void)rc;
-  uint32_t MsgType = (uint32_t)LLI_Terminate;
-  rc = WriteBytes(&MsgType, 4);
-  assert(rc == 4 && "Error writing message type.");
-
   // No data or data size is sent with Terminate
 }
 
-
+bool RemoteTargetExternal::Receive(LLIMessageType Msg) {
-void RemoteTargetExternal::Receive(LLIMessageType ExpectedMsgType) {
+  if (!ReceiveHeader(Msg))
-  int rc;
+    return false;
-  (void)rc;
+  int Unused;
-  uint32_t MsgType;
+  AppendRead(&Unused, 0);
-  rc = ReadBytes(&MsgType, 4);
+  if (!ReceivePayload())
-  assert(rc == 4 && "Error reading message type.");
+    return false;
-  assert(MsgType == (uint32_t)ExpectedMsgType && "Error: received unexpected message type.");
+  ReceiveData.clear();
-
+  Sizes.clear();
-  uint32_t DataSize;
+  return true;
-  rc = ReadBytes(&DataSize, 4);
-  assert(rc == 4 && "Error reading data size.");
-  assert(DataSize == 0 && "Error: unexpected data size.");
 }
 
-void RemoteTargetExternal::Receive(LLIMessageType ExpectedMsgType, int &Data) {
+bool RemoteTargetExternal::Receive(LLIMessageType Msg, int32_t &Data) {
-  uint64_t Temp;
+  if (!ReceiveHeader(Msg))
-  Receive(ExpectedMsgType, Temp);
+    return false;
-  Data = (int)(int64_t)Temp;
+  AppendRead(&Data, 4);
+  if (!ReceivePayload())
+    return false;
+  ReceiveData.clear();
+  Sizes.clear();
+  return true;
 }
 
-void RemoteTargetExternal::Receive(LLIMessageType ExpectedMsgType, uint64_t &Data) {
+bool RemoteTargetExternal::Receive(LLIMessageType Msg, uint64_t &Data) {
-  int rc;
+  if (!ReceiveHeader(Msg))
-  (void)rc;
+    return false;
+  AppendRead(&Data, 8);
+  if (!ReceivePayload())
+    return false;
+  ReceiveData.clear();
+  Sizes.clear();
+  return true;
+}
+
+bool RemoteTargetExternal::ReceiveHeader(LLIMessageType ExpectedMsgType) {
+  assert(ReceiveData.empty() && Sizes.empty() &&
+         "Payload vector not empty to receive header");
+
+  // Message header, with type to follow
   uint32_t MsgType;
-  rc = ReadBytes(&MsgType, 4);
+  if (!ReadBytes(&MsgType, 4)) {
-  assert(rc == 4 && "Error reading message type.");
+    ErrorMsg += ", (RemoteTargetExternal::ReceiveHeader)";
-  assert(MsgType == (uint32_t)ExpectedMsgType && "Error: received unexpected message type.");
+    return false;
+  }
+  if (MsgType != (uint32_t)ExpectedMsgType) {
+    ErrorMsg = "received unexpected message type";
+    ErrorMsg += ". Expecting: ";
+    ErrorMsg += ExpectedMsgType;
+    ErrorMsg += ", Got: ";
+    ErrorMsg += MsgType;
+    return false;
+  }
+  return true;
+}
 
+bool RemoteTargetExternal::ReceivePayload() {
+  assert(!ReceiveData.empty() &&
+         "Payload vector empty to receive");
+  assert(ReceiveData.size() == Sizes.size() &&
+         "Unexpected mismatch between data and size");
+
+  uint32_t TotalSize = 0;
+  for (int I=0, E=Sizes.size(); I < E; I++)
+    TotalSize += Sizes[I];
+
+  // Payload size header
   uint32_t DataSize;
-  rc = ReadBytes(&DataSize, 4);
+  if (!ReadBytes(&DataSize, 4)) {
-  assert(rc == 4 && "Error reading data size.");
+    ErrorMsg += ", invalid data size";
-  assert(DataSize == 8 && "Error: unexpected data size.");
+    return false;
+  }
+  if (DataSize != TotalSize) {
+    ErrorMsg = "unexpected data size";
+    ErrorMsg += ". Expecting: ";
+    ErrorMsg += TotalSize;
+    ErrorMsg += ", Got: ";
+    ErrorMsg += DataSize;
+    return false;
+  }
+  if (DataSize == 0)
+    return true;
 
-  rc = ReadBytes(&Data, 8);
+  // Payload itself
-  assert(DataSize == 8 && "Error: unexpected data.");
+  for (int I=0, E=Sizes.size(); I < E; I++) {
+    if (!ReadBytes(ReceiveData[I], Sizes[I])) {
+      ErrorMsg = "unexpected data while reading message";
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool RemoteTargetExternal::SendHeader(LLIMessageType MsgType) {
+  assert(SendData.empty() && Sizes.empty() &&
+         "Payload vector not empty to send header");
+
+  // Message header, with type to follow
+  if (!WriteBytes(&MsgType, 4)) {
+    ErrorMsg += ", (RemoteTargetExternal::SendHeader)";
+    return false;
+  }
+  return true;
+}
+
+bool RemoteTargetExternal::SendPayload() {
+  assert(!SendData.empty() && !Sizes.empty() &&
+         "Payload vector empty to send");
+  assert(SendData.size() == Sizes.size() &&
+         "Unexpected mismatch between data and size");
+
+  uint32_t TotalSize = 0;
+  for (int I=0, E=Sizes.size(); I < E; I++)
+    TotalSize += Sizes[I];
+
+  // Payload size header
+  if (!WriteBytes(&TotalSize, 4)) {
+    ErrorMsg += ", invalid data size";
+    return false;
+  }
+  if (TotalSize == 0)
+    return true;
+
+  // Payload itself
+  for (int I=0, E=Sizes.size(); I < E; I++) {
+    if (!WriteBytes(SendData[I], Sizes[I])) {
+      ErrorMsg = "unexpected data while writing message";
+      return false;
+    }
+  }
+
+  SendData.clear();
+  Sizes.clear();
+  return true;
+}
+
+void RemoteTargetExternal::AppendWrite(const void *Data, uint32_t Size) {
+  SendData.push_back(Data);
+  Sizes.push_back(Size);
+}
+
+void RemoteTargetExternal::AppendRead(void *Data, uint32_t Size) {
+  ReceiveData.push_back(Data);
+  Sizes.push_back(Size);
 }
 
 #ifdef LLVM_ON_UNIX

tools/lli/RemoteTargetExternal.h

@@ -35,7 +35,7 @@ public:
   /// @param      Alignment Required minimum alignment for allocated space.
   /// @param[out] Address   Remote address of the allocated memory.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool allocateSpace(size_t Size,
                              unsigned Alignment,
@@ -47,7 +47,7 @@ public:
   /// @param      Data      Source address in the host process.
   /// @param      Size      Number of bytes to copy.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool loadData(uint64_t Address, const void *Data, size_t Size);
 
@@ -57,7 +57,7 @@ public:
   /// @param      Data      Source address in the host process.
   /// @param      Size      Number of bytes to copy.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool loadCode(uint64_t Address, const void *Data, size_t Size);
 
@@ -68,7 +68,7 @@ public:
   ///                       process.
   /// @param[out] RetVal    The integer return value of the called function.
   ///
-  /// @returns False on success. On failure, ErrorMsg is updated with
+  /// @returns True on success. On failure, ErrorMsg is updated with
   ///          descriptive text of the encountered error.
   virtual bool executeCode(uint64_t Address, int &RetVal);
 
@@ -80,7 +80,10 @@ public:
   virtual unsigned getPageAlignment() { return 4096; }
 
   /// Start the remote process.
-  virtual void create();
+  ///
+  /// @returns True on success. On failure, ErrorMsg is updated with
+  ///          descriptive text of the encountered error.
+  virtual bool create();
 
   /// Terminate the remote process.
   virtual void stop();
@@ -91,23 +94,37 @@ public:
 private:
   std::string ChildName;
 
-  // This will get filled in as a point to an OS-specific structure.
+  bool SendAllocateSpace(uint32_t Alignment, uint32_t Size);
-  void *ConnectionData;
+  bool SendLoadSection(uint64_t Addr,
-
-  void SendAllocateSpace(uint32_t Alignment, uint32_t Size);
-  void SendLoadSection(uint64_t Addr,
                        const void *Data,
                        uint32_t Size,
                        bool IsCode);
-  void SendExecute(uint64_t Addr);
+  bool SendExecute(uint64_t Addr);
-  void SendTerminate();
+  bool SendTerminate();
 
-  void Receive(LLIMessageType Msg);
+  // High-level wrappers for receiving data
-  void Receive(LLIMessageType Msg, int &Data);
+  bool Receive(LLIMessageType Msg);
-  void Receive(LLIMessageType Msg, uint64_t &Data);
+  bool Receive(LLIMessageType Msg, int32_t &Data);
+  bool Receive(LLIMessageType Msg, uint64_t &Data);
 
-  int WriteBytes(const void *Data, size_t Size);
+  // Lower level target-independent read/write to deal with errors
-  int ReadBytes(void *Data, size_t Size);
+  bool ReceiveHeader(LLIMessageType Msg);
+  bool ReceivePayload();
+  bool SendHeader(LLIMessageType Msg);
+  bool SendPayload();
+
+  // Functions to append/retrieve data from the payload
+  SmallVector<const void *, 2> SendData;
+  SmallVector<void *, 1> ReceiveData; // Future proof
+  SmallVector<int, 2> Sizes;
+  void AppendWrite(const void *Data, uint32_t Size);
+  void AppendRead(void *Data, uint32_t Size);
+
+  // This will get filled in as a point to an OS-specific structure.
+  void *ConnectionData;
+
+  bool WriteBytes(const void *Data, size_t Size);
+  bool ReadBytes(void *Data, size_t Size);
   void Wait();
 };
 

tools/lli/RemoteTargetMessage.h

@@ -26,20 +26,60 @@ namespace llvm {
 // only here for testing purposes and is therefore intended to be the simplest
 // implementation that will work.  It is assumed that the parent and child
 // share characteristics like endianness.
+//
+// Quick description of the protocol:
+//
+// { Header + Payload Size + Payload }
+//
+// The protocol message consist of a header, the payload size (which can be
+// zero), and the payload itself. The payload can contain any number of items,
+// and the size has to be the sum of them all. Each end is responsible for
+// reading/writing the correct number of items with the correct sizes.
+//
+// The current four known exchanges are:
+//
+//  * Allocate Space:
+//   Parent: { LLI_AllocateSpace, 8, Alignment, Size }
+//    Child: { LLI_AllocationResult, 8, Address }
+//
+//  * Load Data:
+//   Parent: { LLI_LoadDataSection, 8+Size, Address, Data }
+//    Child: { LLI_LoadComplete, 4, StatusCode }
+//
+//  * Load Code:
+//   Parent: { LLI_LoadCodeSection, 8+Size, Address, Code }
+//    Child: { LLI_LoadComplete, 4, StatusCode }
+//
+//  * Execute Code:
+//   Parent: { LLI_Execute, 8, Address }
+//    Child: { LLI_ExecutionResult, 4, Result }
+//
+// It is the responsibility of either side to check for correct headers,
+// sizes and payloads, since any inconsistency would misalign the pipe, and
+// result in data corruption.
 
 enum LLIMessageType {
   LLI_Error = -1,
   LLI_ChildActive = 0,        // Data = not used
-  LLI_AllocateSpace,          // Data = struct { uint_32t Align, uint_32t Size }
+  LLI_AllocateSpace,          // Data = struct { uint32_t Align, uint_32t Size }
-  LLI_AllocationResult,       // Data = uint64_t AllocAddress (in Child memory space)
+  LLI_AllocationResult,       // Data = uint64_t Address (child memory space)
-  LLI_LoadCodeSection,        // Data = uint32_t Addr, followed by section contests
+
-  LLI_LoadDataSection,        // Data = uint32_t Addr, followed by section contents
+  LLI_LoadCodeSection,        // Data = uint64_t Address, void * SectionData
-  LLI_LoadComplete,           // Data = not used
+  LLI_LoadDataSection,        // Data = uint64_t Address, void * SectionData
-  LLI_Execute,                // Data = Address of function to execute
+  LLI_LoadResult,             // Data = uint32_t LLIMessageStatus
-  LLI_ExecutionResult,        // Data = uint64_t Result
+
+  LLI_Execute,                // Data = uint64_t Address
+  LLI_ExecutionResult,        // Data = uint32_t Result
+
   LLI_Terminate               // Data = not used
 };
 
+enum LLIMessageStatus {
+  LLI_Status_Success = 0,     // Operation succeeded
+  LLI_Status_NotAllocated,    // Address+Size not allocated in child space
+  LLI_Status_IncompleteMsg    // Size received doesn't match request
+};
+
 } // end namespace llvm
 
 #endif

tools/lli/Unix/RemoteTargetExternal.inc

@@ -30,7 +30,7 @@ struct ConnectionData_t {
 
 namespace llvm {
 
-void RemoteTargetExternal::create() {
+bool RemoteTargetExternal::create() {
   int PipeFD[2][2];
   pid_t ChildPID;
 
@@ -73,16 +73,53 @@ void RemoteTargetExternal::create() {
     // Store the parent ends of the pipes
     ConnectionData = (void*)new ConnectionData_t(PipeFD[1][0], PipeFD[0][1]);
 
-    Receive(LLI_ChildActive);
+    // We must get Ack from the client (blocking read)
+    if (!Receive(LLI_ChildActive)) {
+      ErrorMsg += ", (RemoteTargetExternal::create) - Stopping process!";
+      stop();
+      return false;
+    }
+  }
+  return true;
+}
+
+static void ReportError(int rc, size_t Size, std::string &ErrorMsg) {
+  if (rc == -1) {
+    if (errno == EPIPE)
+      ErrorMsg += "pipe closed";
+    else if (errno == EINTR)
+      ErrorMsg += "interrupted";
+    else
+      ErrorMsg += "file descriptor error";
+  } else {
+    char Number[10] = { 0 };
+    ErrorMsg += "Expecting ";
+    sprintf(Number, "%d", (uint32_t)Size);
+    ErrorMsg += Number;
+    ErrorMsg += " bytes, Got ";
+    sprintf(Number, "%d", rc);
+    ErrorMsg += Number;
   }
 }
 
-int RemoteTargetExternal::WriteBytes(const void *Data, size_t Size) {
+bool RemoteTargetExternal::WriteBytes(const void *Data, size_t Size) {
-  return write(((ConnectionData_t*)ConnectionData)->OutputPipe, Data, Size);
+  int rc = write(((ConnectionData_t*)ConnectionData)->OutputPipe, Data, Size);
+  if (rc != -1 && (size_t)rc == Size)
+    return true;
+
+  ErrorMsg = "WriteBytes: ";
+  ReportError(rc, Size, ErrorMsg);
+  return false;
 }
 
-int RemoteTargetExternal::ReadBytes(void *Data, size_t Size) {
+bool RemoteTargetExternal::ReadBytes(void *Data, size_t Size) {
-  return read(((ConnectionData_t*)ConnectionData)->InputPipe, Data, Size);
+  int rc = read(((ConnectionData_t*)ConnectionData)->InputPipe, Data, Size);
+  if (rc != -1 && (size_t)rc == Size)
+    return true;
+
+  ErrorMsg = "ReadBytes: ";
+  ReportError(rc, Size, ErrorMsg);
+  return false;
 }
 
 void RemoteTargetExternal::Wait() {

tools/lli/lli.cpp

@@ -685,7 +685,10 @@ int main(int argc, char **argv, char * const *envp) {
     MM->setRemoteTarget(Target.get());
 
     // Create the remote target.
-    Target->create();
+    if (!Target->create()) {
+      errs() << "ERROR: " << Target->getErrorMsg() << "\n";
+      return EXIT_FAILURE;
+    }
 
     // Since we're executing in a (at least simulated) remote address space,
     // we can't use the ExecutionEngine::runFunctionAsMain(). We have to
@@ -702,7 +705,7 @@ int main(int argc, char **argv, char * const *envp) {
     DEBUG(dbgs() << "Executing '" << EntryFn->getName() << "' at 0x"
                  << format("%llx", Entry) << "\n");
 
-    if (Target->executeCode(Entry, Result))
+    if (!Target->executeCode(Entry, Result))
       errs() << "ERROR: " << Target->getErrorMsg() << "\n";
 
     // Like static constructors, the remote target MCJIT support doesn't handle