From 407c721ceb93863b2cb3851a6aa7686f31657e6b Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 14 Mar 2022 11:38:04 -0700 Subject: [PATCH] [Support] Change zlib::compress to return void 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. Now that there is no other error type, we can replace the return type with void and simplify call sites. Reviewed By: ikudrin Differential Revision: https://reviews.llvm.org/D121512 --- .../clangd/index/Serialization.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 14 ++++------- llvm/include/llvm/ProfileData/SampleProf.h | 1 - llvm/include/llvm/Support/Compression.h | 4 ++-- llvm/lib/MC/ELFObjectWriter.cpp | 9 ++----- llvm/lib/ObjCopy/ELF/ELFObject.cpp | 24 ++++--------------- llvm/lib/ObjCopy/ELF/ELFObject.h | 2 +- .../Coverage/CoverageMappingWriter.cpp | 8 ++----- llvm/lib/ProfileData/InstrProf.cpp | 8 ++----- llvm/lib/ProfileData/SampleProf.cpp | 2 -- llvm/lib/ProfileData/SampleProfWriter.cpp | 6 ++--- llvm/lib/Support/Compression.cpp | 12 ++++++---- llvm/unittests/Support/CompressionTest.cpp | 7 ++---- 13 files changed, 31 insertions(+), 68 deletions(-) diff --git a/clang-tools-extra/clangd/index/Serialization.cpp b/clang-tools-extra/clangd/index/Serialization.cpp index 8ceecb607236..f7045f0be3fd 100644 --- a/clang-tools-extra/clangd/index/Serialization.cpp +++ b/clang-tools-extra/clangd/index/Serialization.cpp @@ -192,7 +192,7 @@ public: } 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 { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 63fcc1eaa550..69c9d61c5227 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2007,15 +2007,11 @@ static void emitBlob(llvm::BitstreamWriter &Stream, StringRef Blob, // 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}; + Stream.EmitRecordWithBlob(SLocBufferBlobCompressedAbbrv, Record, + CompressedBuffer); + return; } RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB}; diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h index 23052ae30032..178bfced88a0 100644 --- a/llvm/include/llvm/ProfileData/SampleProf.h +++ b/llvm/include/llvm/ProfileData/SampleProf.h @@ -55,7 +55,6 @@ enum class sampleprof_error { not_implemented, counter_overflow, ostream_seek_unsupported, - compress_failed, uncompress_failed, zlib_unavailable, hash_mismatch diff --git a/llvm/include/llvm/Support/Compression.h b/llvm/include/llvm/Support/Compression.h index 5bc0e56913fe..e6f898229412 100644 --- a/llvm/include/llvm/Support/Compression.h +++ b/llvm/include/llvm/Support/Compression.h @@ -29,8 +29,8 @@ static constexpr int BestSizeCompression = 9; bool isAvailable(); -Error compress(StringRef InputBuffer, SmallVectorImpl &CompressedBuffer, - int Level = DefaultCompression); +void compress(StringRef InputBuffer, SmallVectorImpl &CompressedBuffer, + int Level = DefaultCompression); Error uncompress(StringRef InputBuffer, char *UncompressedBuffer, size_t &UncompressedSize); diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp index abcd3bb2b849..00d18cd35ab4 100644 --- a/llvm/lib/MC/ELFObjectWriter.cpp +++ b/llvm/lib/MC/ELFObjectWriter.cpp @@ -864,13 +864,8 @@ void ELFWriter::writeSectionData(const MCAssembler &Asm, MCSection &Sec, Asm.writeSectionData(VecOS, &Section, Layout); SmallVector 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, diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.cpp b/llvm/lib/ObjCopy/ELF/ELFObject.cpp index 522804acac78..cb5c82c2e337 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObject.cpp +++ b/llvm/lib/ObjCopy/ELF/ELFObject.cpp @@ -548,13 +548,7 @@ Error ELFSectionWriter::visit(const CompressedSection &Sec) { Expected 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::create(ArrayRef CompressedData, @@ -564,20 +558,12 @@ CompressedSection::create(ArrayRef CompressedData, } 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(OriginalData.data()), - OriginalData.size()), - CompressedData)) { - OutErr = createStringError(llvm::errc::invalid_argument, - "'" + Name + "': " + toString(std::move(Err))); - return; - } + zlib::compress(StringRef(reinterpret_cast(OriginalData.data()), + OriginalData.size()), + CompressedData); size_t ChdrSize; if (CompressionType == DebugCompressionType::GNU) { diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.h b/llvm/lib/ObjCopy/ELF/ELFObject.h index 37134c849a15..80f043d8d632 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObject.h +++ b/llvm/lib/ObjCopy/ELF/ELFObject.h @@ -561,7 +561,7 @@ public: private: CompressedSection(const SectionBase &Sec, - DebugCompressionType CompressionType, Error &Err); + DebugCompressionType CompressionType); CompressedSection(ArrayRef CompressedData, uint64_t DecompressedSize, uint64_t DecompressedAlign); }; diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp index ceb2d7dcb5b9..781a2901dbb9 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp @@ -49,12 +49,8 @@ void CoverageFilenamesSectionWriter::write(raw_ostream &OS, bool Compress) { 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) + zlib::compress(FilenamesStr, CompressedStr, zlib::BestSizeCompression); // ::= // diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 0a0ce7604a29..48ac5ce0d607 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -467,12 +467,8 @@ Error collectPGOFuncNameStrings(ArrayRef NameStrs, } SmallString<128> CompressedNameStrings; - Error E = zlib::compress(StringRef(UncompressedNameStrings), - CompressedNameStrings, zlib::BestSizeCompression); - if (E) { - consumeError(std::move(E)); - return make_error(instrprof_error::compress_failed); - } + zlib::compress(StringRef(UncompressedNameStrings), CompressedNameStrings, + zlib::BestSizeCompression); return WriteStringToResult(CompressedNameStrings.size(), CompressedNameStrings); diff --git a/llvm/lib/ProfileData/SampleProf.cpp b/llvm/lib/ProfileData/SampleProf.cpp index 841cda5728bf..710751cf0b04 100644 --- a/llvm/lib/ProfileData/SampleProf.cpp +++ b/llvm/lib/ProfileData/SampleProf.cpp @@ -85,8 +85,6 @@ class SampleProfErrorCategoryType : public std::error_category { 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: diff --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp index f632fb3b27f5..5ad3e945fefe 100644 --- a/llvm/lib/ProfileData/SampleProfWriter.cpp +++ b/llvm/lib/ProfileData/SampleProfWriter.cpp @@ -86,10 +86,8 @@ std::error_code SampleProfileWriterExtBinaryBase::compressAndOutput() { 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, + zlib::BestSizeCompression); encodeULEB128(UncompressedStrings.size(), OS); encodeULEB128(CompressedStrings.size(), OS); OS << CompressedStrings.str(); diff --git a/llvm/lib/Support/Compression.cpp b/llvm/lib/Support/Compression.cpp index ccf6ef4bb662..983a6348bbe4 100644 --- a/llvm/lib/Support/Compression.cpp +++ b/llvm/lib/Support/Compression.cpp @@ -46,18 +46,20 @@ static StringRef convertZlibCodeToString(int Code) { bool zlib::isAvailable() { return true; } -Error zlib::compress(StringRef InputBuffer, - SmallVectorImpl &CompressedBuffer, int Level) { +void zlib::compress(StringRef InputBuffer, + SmallVectorImpl &CompressedBuffer, int Level) { 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, @@ -87,8 +89,8 @@ uint32_t zlib::crc32(StringRef Buffer) { #else bool zlib::isAvailable() { return false; } -Error zlib::compress(StringRef InputBuffer, - SmallVectorImpl &CompressedBuffer, int Level) { +void zlib::compress(StringRef InputBuffer, + SmallVectorImpl &CompressedBuffer, int Level) { llvm_unreachable("zlib::compress is unavailable"); } Error zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer, diff --git a/llvm/unittests/Support/CompressionTest.cpp b/llvm/unittests/Support/CompressionTest.cpp index 51723898e950..911069f39f11 100644 --- a/llvm/unittests/Support/CompressionTest.cpp +++ b/llvm/unittests/Support/CompressionTest.cpp @@ -27,13 +27,10 @@ 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);