[PATCH] D124746: Make BinaryStreamWriter::padToAlignment write blocks vs bytes.

[Stella Laurenzo via Phabricator via llvm-commits]

stellaraccident created this revision.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: All.
stellaraccident requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

While I think this is a performance improvement over the original, this actually fixes a correctness issue: For an appendable underlying stream, padToAlignment would fail if the additional padding would have caused the stream to grow since it was doing its own check on bounds. By deferring to the regular writeArray method this takes the same path as everything else, which does the correct bounds check in WritableBinaryStreamRef::checkOffsetForWrite (i.e. skips the extension check if BSF_Append is set). I had started to fix the existing bounds check in BinaryStreamWriter but deferred to this because it layered better and is more efficient/consistent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124746

Files:
  llvm/lib/Support/BinaryStreamWriter.cpp


Index: llvm/lib/Support/BinaryStreamWriter.cpp
===================================================================
--- llvm/lib/Support/BinaryStreamWriter.cpp
+++ llvm/lib/Support/BinaryStreamWriter.cpp
@@ -13,6 +13,8 @@
 #include "llvm/Support/BinaryStreamRef.h"
 #include "llvm/Support/LEB128.h"
 
+#include <cstring>
+
 using namespace llvm;
 
 BinaryStreamWriter::BinaryStreamWriter(WritableBinaryStreamRef Ref)
@@ -94,10 +96,12 @@
 
 Error BinaryStreamWriter::padToAlignment(uint32_t Align) {
   uint64_t NewOffset = alignTo(Offset, Align);
-  if (NewOffset > getLength())
-    return make_error<BinaryStreamError>(stream_error_code::stream_too_short);
+  const uint64_t ZerosSize = 64;
+  char Zeros[ZerosSize];
+  std::memset(Zeros, 0, ZerosSize);
   while (Offset < NewOffset)
-    if (auto EC = writeInteger('\0'))
-      return EC;
+    if (auto E = writeArray(
+            ArrayRef<char>(Zeros, std::min(ZerosSize, NewOffset - Offset))))
+      return E;
   return Error::success();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124746.426320.patch
Type: text/x-patch
Size: 998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220502/61c20e44/attachment.bin>