[Support] Change zlib::compress to return void

[MaskRay]

Summary

With a sufficiently large output buffer, the only failure is Z_MEM_ERROR.
Check it and call the noreturn report_bad_alloc_error if applicable.
resize_for_overwrite may call report_bad_alloc_error as well.

Since there is now no other error type, we can replace the return type
with void and simplify call sites.

[MaskRay]

Next, I'll change zlib::compress to return void and simplify the ~7 call sites.

[joerg]

compressBound gives the maximum possible size for the output, so of course it can be larger than the input. In combination with resize_for_overwrite, that should be cheaper than possible multiple copies for the loop if there is even a slight chance that the data doesn't compress that well. The code complexity is certainly much higher. The only expected error from compress2 here is Z_MEM_ERROR when it can't allocate the dictionary etc. As such, we could convert that one into a fatal error easily.

[MaskRay]

In D121512#3377419, @joerg wrote:

compressBound gives the maximum possible size for the output, so of course it can be larger than the input. In combination with resize_for_overwrite, that should be cheaper than possible multiple copies for the loop if there is even a slight chance that the data doesn't compress that well. The code complexity is certainly much higher. The only expected error from compress2 here is Z_MEM_ERROR when it can't allocate the dictionary etc. As such, we could convert that one into a fatal error easily.

The chance that 0.5*N needs reallocation is very slow (for some large executables, all input .debug_* sections can compress to 55%-. Most are smaller than 40%). In most cases the allocated buffer will save more than one half.

[joerg]

But that doesn't really matter much as long as it is uninitialized virtual memory? This is almost always used for a very short time as well.

[MaskRay]

In D121512#3377511, @joerg wrote:

But that doesn't really matter much as long as it is uninitialized virtual memory? This is almost always used for a very short time as well.

I agree this complexity is unneeded. Then abandon this patch and replace return Res ? createError(convertZlibCodeToString(Res)) : Error::success(); with something like

if (Res == Z_MEM_ERROR)
  report_bad_alloc_error("Allocation failed"); // like llvm/include/llvm/Support/MemAlloc.h safe_malloc, which may be called by resize_for_overwrite if oom
return Error::success();

?

[joerg]

Explicit check for Z_MEM_ERROR and assert that it is otherwise Z_OK. Other return values would point to a programming error in zlib.

[MaskRay]

repurpose the patch to return void

[joerg]

LGTM

[ikudrin]

llvm/lib/ObjCopy/ELF/ELFObject.cpp

561

OutErr can now be removed to further simplify the calling code.

[MaskRay]

Simplify CompressedSection::CompressedSection in llvm-objcopy.

CompressedSection::create can be further simplified, but I'll leave that as a future change.

[ikudrin]

LGTM

clang-tools-extra/clangd/index/Serialization.cpp

void finalize(llvm::raw_ostream &OS) {
 
     std::string RawTable;
     for (llvm::StringRef S : Sorted) {
       RawTable.append(std::string(S));
       RawTable.push_back(0);
     }
     if (llvm::zlib::isAvailable()) {
       llvm::SmallString<1> Compressed;
-      llvm::cantFail(llvm::zlib::compress(RawTable, Compressed));
+      llvm::zlib::compress(RawTable, Compressed);
       write32(RawTable.size(), OS);
       OS << Compressed;
     } else {
       write32(0, OS); // No compression.
       OS << RawTable;
     }
   }
   // Get the ID of an string, which must be interned. Table must be finalized.

clang/lib/Serialization/ASTWriter.cpp

static void emitBlob(llvm::BitstreamWriter &Stream, StringRef Blob,
                      unsigned SLocBufferBlobCompressedAbbrv,
                      unsigned SLocBufferBlobAbbrv) {
   using RecordDataType = ASTWriter::RecordData::value_type;
 
   // Compress the buffer if possible. We expect that almost all PCM
   // consumers will not want its contents.
   SmallString<0> CompressedBuffer;
   if (llvm::zlib::isAvailable()) {
-    llvm::Error E = llvm::zlib::compress(Blob.drop_back(1), CompressedBuffer);
-    if (!E) {
-      RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB_COMPRESSED,
-                                 Blob.size() - 1};
-      Stream.EmitRecordWithBlob(SLocBufferBlobCompressedAbbrv, Record,
-                                CompressedBuffer);
-      return;
-    }
-    llvm::consumeError(std::move(E));
-  }
+    llvm::zlib::compress(Blob.drop_back(1), CompressedBuffer);
+    RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB_COMPRESSED, Blob.size() - 1};
 
   RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB};
   Stream.EmitRecordWithBlob(SLocBufferBlobAbbrv, Record, Blob);
 }
 
 /// Writes the block containing the serialized form of the
 /// source manager.
 ///

llvm/include/llvm/ProfileData/SampleProf.h

enum class sampleprof_error {
   truncated,
   malformed,
   unrecognized_format,
   unsupported_writing_format,
   truncated_name_table,
   not_implemented,
   counter_overflow,
   ostream_seek_unsupported,
-  compress_failed,
   uncompress_failed,
   zlib_unavailable,
   hash_mismatch
 };
 
 inline std::error_code make_error_code(sampleprof_error E) {
   return std::error_code(static_cast<int>(E), sampleprof_category());
 }

llvm/include/llvm/Support/Compression.h

 
 static constexpr int NoCompression = 0;
 static constexpr int BestSpeedCompression = 1;
 static constexpr int DefaultCompression = 6;
 static constexpr int BestSizeCompression = 9;
 
 bool isAvailable();
 
-Error compress(StringRef InputBuffer, SmallVectorImpl<char> &CompressedBuffer,
-               int Level = DefaultCompression);
+void compress(StringRef InputBuffer, SmallVectorImpl<char> &CompressedBuffer,
 
 Error uncompress(StringRef InputBuffer, char *UncompressedBuffer,
                  size_t &UncompressedSize);
 
 Error uncompress(StringRef InputBuffer,
                  SmallVectorImpl<char> &UncompressedBuffer,
                  size_t UncompressedSize);
 
 uint32_t crc32(StringRef Buffer);
 
 }  // End of namespace zlib
 
 } // End of namespace llvm
 
 #endif

llvm/lib/MC/ELFObjectWriter.cpp

assert((MAI->compressDebugSections() == DebugCompressionType::Z ||
           MAI->compressDebugSections() == DebugCompressionType::GNU) &&
          "expected zlib or zlib-gnu style compression");
 
   SmallVector<char, 128> UncompressedData;
   raw_svector_ostream VecOS(UncompressedData);
   Asm.writeSectionData(VecOS, &Section, Layout);
 
   SmallVector<char, 128> CompressedContents;
-  if (Error E = zlib::compress(
-          StringRef(UncompressedData.data(), UncompressedData.size()),
-          CompressedContents)) {
-    consumeError(std::move(E));
-    W.OS << UncompressedData;
-    return;
-  }
+  zlib::compress(StringRef(UncompressedData.data(), UncompressedData.size()),
+                 CompressedContents);
 
   bool ZlibStyle = MAI->compressDebugSections() == DebugCompressionType::Z;
   if (!maybeWriteCompression(UncompressedData.size(), CompressedContents,
                              ZlibStyle, Sec.getAlignment())) {
     W.OS << UncompressedData;
     return;
   }
 

llvm/lib/ObjCopy/ELF/ELFObject.h

public:
 
   static bool classof(const SectionBase *S) {
     return (S->OriginalFlags & ELF::SHF_COMPRESSED) ||
            (StringRef(S->Name).startswith(".zdebug"));
   }
 
 private:
   CompressedSection(const SectionBase &Sec,
-                    DebugCompressionType CompressionType, Error &Err);
+                    DebugCompressionType CompressionType);
   CompressedSection(ArrayRef<uint8_t> CompressedData, uint64_t DecompressedSize,
                     uint64_t DecompressedAlign);
 };
 
 class DecompressedSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
 public:

llvm/lib/ObjCopy/ELF/ELFObject.cpp

Error ELFSectionWriter<ELFT>::visit(const CompressedSection &Sec) {
 
   std::copy(Sec.CompressedData.begin(), Sec.CompressedData.end(), Buf);
   return Error::success();
 }
 
 Expected<CompressedSection>
 CompressedSection::create(const SectionBase &Sec,
                           DebugCompressionType CompressionType) {
-  Error Err = Error::success();
-  CompressedSection Section(Sec, CompressionType, Err);
-
-  if (Err)
-    return std::move(Err);
-
-  return Section;
+  return CompressedSection(Sec, CompressionType);
 }
 Expected<CompressedSection>
 CompressedSection::create(ArrayRef<uint8_t> CompressedData,
                           uint64_t DecompressedSize,
                           uint64_t DecompressedAlign) {
   return CompressedSection(CompressedData, DecompressedSize, DecompressedAlign);
 }
 
 CompressedSection::CompressedSection(const SectionBase &Sec,
-                                     DebugCompressionType CompressionType,
-                                     Error &OutErr)
+                                     DebugCompressionType CompressionType)
     : SectionBase(Sec), CompressionType(CompressionType),
       DecompressedSize(Sec.OriginalData.size()), DecompressedAlign(Sec.Align) {
-  ErrorAsOutParameter EAO(&OutErr);
-
-  if (Error Err = zlib::compress(
-          StringRef(reinterpret_cast<const char *>(OriginalData.data()),
+  zlib::compress(StringRef(reinterpret_cast<const char *>(OriginalData.data()),
                            OriginalData.size()),
-          CompressedData)) {
-    OutErr = createStringError(llvm::errc::invalid_argument,
-                               "'" + Name + "': " + toString(std::move(Err)));
-    return;
-  }
+                 CompressedData);
 
   size_t ChdrSize;
   if (CompressionType == DebugCompressionType::GNU) {
     Name = ".z" + Sec.Name.substr(1);
     ChdrSize = sizeof("ZLIB") - 1 + sizeof(uint64_t);
   } else {
     Flags |= ELF::SHF_COMPRESSED;
     ChdrSize =

llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp

for (const auto &Filename : Filenames) {
       encodeULEB128(Filename.size(), FilenamesOS);
       FilenamesOS << Filename;
     }
   }
 
   SmallString<128> CompressedStr;
   bool doCompression =
       Compress && zlib::isAvailable() && DoInstrProfNameCompression;
-  if (doCompression) {
-    auto E =
-        zlib::compress(FilenamesStr, CompressedStr, zlib::BestSizeCompression);
-    if (E)
-      report_bad_alloc_error("Failed to zlib compress coverage data");
-  }
+  if (doCompression)
 
   // ::= <num-filenames>
   //     <uncompressed-len>
   //     <compressed-len-or-zero>
   //     (<compressed-filenames> | <uncompressed-filenames>)
   encodeULEB128(Filenames.size(), OS);
   encodeULEB128(FilenamesStr.size(), OS);
   encodeULEB128(doCompression ? CompressedStr.size() : 0U, OS);

llvm/lib/ProfileData/InstrProf.cpp

auto WriteStringToResult = [&](size_t CompressedLen, StringRef InputStr) {
     return Error::success();
   };
 
   if (!doCompression) {
     return WriteStringToResult(0, UncompressedNameStrings);
   }
 
   SmallString<128> CompressedNameStrings;
-  Error E = zlib::compress(StringRef(UncompressedNameStrings),
-                           CompressedNameStrings, zlib::BestSizeCompression);
-  if (E) {
-    consumeError(std::move(E));
-    return make_error<InstrProfError>(instrprof_error::compress_failed);
-  }
+  zlib::compress(StringRef(UncompressedNameStrings), CompressedNameStrings,
+                 zlib::BestSizeCompression);
 
   return WriteStringToResult(CompressedNameStrings.size(),
                              CompressedNameStrings);
 }
 
 StringRef getPGOFuncNameVarInitializer(GlobalVariable *NameVar) {
   auto *Arr = cast<ConstantDataArray>(NameVar->getInitializer());
   StringRef NameStr =

llvm/lib/ProfileData/SampleProf.cpp

std::string message(int IE) const override {
     case sampleprof_error::truncated_name_table:
       return "Truncated function name table";
     case sampleprof_error::not_implemented:
       return "Unimplemented feature";
     case sampleprof_error::counter_overflow:
       return "Counter overflow";
     case sampleprof_error::ostream_seek_unsupported:
       return "Ostream does not support seek";
-    case sampleprof_error::compress_failed:
-      return "Compress failure";
     case sampleprof_error::uncompress_failed:
       return "Uncompress failure";
     case sampleprof_error::zlib_unavailable:
       return "Zlib is unavailable";
     case sampleprof_error::hash_mismatch:
       return "Function hash mismatch";
     }
     llvm_unreachable("A value of sampleprof_error has no message.");

llvm/lib/ProfileData/SampleProfWriter.cpp

std::error_code SampleProfileWriterExtBinaryBase::compressAndOutput() {
   if (!llvm::zlib::isAvailable())
     return sampleprof_error::zlib_unavailable;
   std::string &UncompressedStrings =
       static_cast<raw_string_ostream *>(LocalBufStream.get())->str();
   if (UncompressedStrings.size() == 0)
     return sampleprof_error::success;
   auto &OS = *OutputStream;
   SmallString<128> CompressedStrings;
-  llvm::Error E = zlib::compress(UncompressedStrings, CompressedStrings,
-                                 zlib::BestSizeCompression);
-  if (E)
-    return sampleprof_error::compress_failed;
+  zlib::compress(UncompressedStrings, CompressedStrings,
   encodeULEB128(UncompressedStrings.size(), OS);
   encodeULEB128(CompressedStrings.size(), OS);
   OS << CompressedStrings.str();
   UncompressedStrings.clear();
   return sampleprof_error::success;
 }
 
 /// Add a new section into section header table given the section type

llvm/lib/Support/Compression.cpp

static StringRef convertZlibCodeToString(int Code) {
   case Z_OK:
   default:
     llvm_unreachable("unknown or unexpected zlib status code");
   }
 }
 
 bool zlib::isAvailable() { return true; }
 
-Error zlib::compress(StringRef InputBuffer,
-                     SmallVectorImpl<char> &CompressedBuffer, int Level) {
+void zlib::compress(StringRef InputBuffer,
   unsigned long CompressedSize = ::compressBound(InputBuffer.size());
   CompressedBuffer.resize_for_overwrite(CompressedSize);
   int Res =
       ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,
                   (const Bytef *)InputBuffer.data(), InputBuffer.size(), Level);
+  if (Res == Z_MEM_ERROR)
+    report_bad_alloc_error("Allocation failed");
+  assert(Res == Z_OK);
   // Tell MemorySanitizer that zlib output buffer is fully initialized.
   // This avoids a false report when running LLVM with uninstrumented ZLib.
   __msan_unpoison(CompressedBuffer.data(), CompressedSize);
   CompressedBuffer.truncate(CompressedSize);
-  return Res ? createError(convertZlibCodeToString(Res)) : Error::success();
 }
 
 Error zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
                        size_t &UncompressedSize) {
   int Res =
       ::uncompress((Bytef *)UncompressedBuffer, (uLongf *)&UncompressedSize,
                    (const Bytef *)InputBuffer.data(), InputBuffer.size());
   // Tell MemorySanitizer that zlib output buffer is fully initialized.

 }
 
 uint32_t zlib::crc32(StringRef Buffer) {
   return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
 }
 
 #else
 bool zlib::isAvailable() { return false; }
-Error zlib::compress(StringRef InputBuffer,
-                     SmallVectorImpl<char> &CompressedBuffer, int Level) {
+void zlib::compress(StringRef InputBuffer,
   llvm_unreachable("zlib::compress is unavailable");
 }
 Error zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
                        size_t &UncompressedSize) {
   llvm_unreachable("zlib::uncompress is unavailable");
 }
 Error zlib::uncompress(StringRef InputBuffer,
                        SmallVectorImpl<char> &UncompressedBuffer,
                        size_t UncompressedSize) {
   llvm_unreachable("zlib::uncompress is unavailable");
 }
 uint32_t zlib::crc32(StringRef Buffer) {
   llvm_unreachable("zlib::crc32 is unavailable");
 }
 #endif

llvm/unittests/Support/CompressionTest.cpp

 namespace {
 
 #if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
   SmallString<32> Uncompressed;
 
-  Error E = zlib::compress(Input, Compressed, Level);
-  EXPECT_FALSE(E);
-  consumeError(std::move(E));
+  zlib::compress(Input, Compressed, Level);
 
   // Check that uncompressed buffer is the same as original.
-  E = zlib::uncompress(Compressed, Uncompressed, Input.size());
-  EXPECT_FALSE(E);
+  Error E = zlib::uncompress(Compressed, Uncompressed, Input.size());
   consumeError(std::move(E));
 
   EXPECT_EQ(Input, Uncompressed);
   if (Input.size() > 0) {
     // Uncompression fails if expected length is too short.
     E = zlib::uncompress(Compressed, Uncompressed, Input.size() - 1);
     EXPECT_EQ("zlib error: Z_BUF_ERROR", llvm::toString(std::move(E)));
   }