From 66ddf592b4d5d997ea00cb28ee602b3011af7074 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 10:04:22 -0800 Subject: [PATCH 1/2] SaveState: Use malloc to avoid Android OOM crash. We have exceptions disabled on Android, which is exactly where we need the OOM check most. --- Common/ChunkFile.cpp | 39 +++++++++++++++++++++------------------ Common/ChunkFile.h | 13 +++---------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/Common/ChunkFile.cpp b/Common/ChunkFile.cpp index ecf422b0f9..5f380347fd 100644 --- a/Common/ChunkFile.cpp +++ b/Common/ChunkFile.cpp @@ -15,6 +15,7 @@ // Official SVN repository and contact information can be found at // http://code.google.com/p/dolphin-emu/ +#include #include #include @@ -271,18 +272,23 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, INFO_LOG(SAVESTATE, "ChunkReader: Writing %s", filename.c_str()); File::IOFile pFile(filename, "wb"); - if (!pFile) - { + if (!pFile) { ERROR_LOG(SAVESTATE, "ChunkReader: Error opening file for write"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - bool compress = true; + // Make sure we can allocate a buffer to compress before compressing. + size_t comp_len = snappy_max_compressed_length(sz); + u8 *compressed_buffer = (u8 *)malloc(comp_len); + if (!compressed_buffer) { + ERROR_LOG(SAVESTATE, "ChunkReader: Unable to allocate compressed buffer"); + // We'll save uncompressed. Better than not saving... + } // Create header SChunkHeader header; - header.Compress = compress ? 1 : 0; + header.Compress = compressed_buffer ? 1 : 0; header.Revision = REVISION_CURRENT; header.ExpectedSize = (u32)sz; header.UncompressedSize = (u32)sz; @@ -293,12 +299,11 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, truncate_cpy(titleFixed, title.c_str()); // Write to file - if (compress) { - size_t comp_len = snappy_max_compressed_length(sz); - u8 *compressed_buffer = new u8[comp_len]; + if (compressed_buffer) { snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &comp_len); - delete [] buffer; + free(buffer); header.ExpectedSize = (u32)comp_len; + if (!pFile.WriteArray(&header, 1)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); return ERROR_BAD_FILE; @@ -307,27 +312,25 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); return ERROR_BAD_FILE; } - if (!pFile.WriteBytes(&compressed_buffer[0], comp_len)) { + if (!pFile.WriteBytes(compressed_buffer, comp_len)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); return ERROR_BAD_FILE; } else { INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)comp_len); } - delete [] compressed_buffer; + free(compressed_buffer); } else { - if (!pFile.WriteArray(&header, 1)) - { + if (!pFile.WriteArray(&header, 1)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - if (!pFile.WriteBytes(&buffer[0], sz)) - { + if (!pFile.WriteBytes(&buffer[0], sz)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing data"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - delete [] buffer; + free(buffer); } INFO_LOG(SAVESTATE, "ChunkReader: Done writing %s", filename.c_str()); diff --git a/Common/ChunkFile.h b/Common/ChunkFile.h index 1820dc2192..6fc76ddb29 100644 --- a/Common/ChunkFile.h +++ b/Common/ChunkFile.h @@ -29,6 +29,7 @@ // + Sections can be versioned for backwards/forwards compatibility // - Serialization code for anything complex has to be manually written. +#include #include #include #include @@ -646,17 +647,9 @@ public: { // Get data size_t const sz = MeasurePtr(_class); - u8 *buffer = nullptr; -#if PPSSPP_PLATFORM(ANDROID) - buffer = new u8[sz]; -#else - try { - buffer = new u8[sz]; - } - catch (std::bad_alloc e) { + u8 *buffer = (u8 *)malloc(sz); + if (!buffer) return ERROR_BAD_ALLOC; - } -#endif Error error = SavePtr(buffer, _class); // SaveFile takes ownership of buffer From 349b36f000538ff29b15a97cd837004f436e5a9d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 10:17:41 -0800 Subject: [PATCH 2/2] SaveState: Correct free on error and uncomp format. The uncompressed path had diverged, so refactor to avoid this in the future. Also, some errors weren't freeing all buffers. --- Common/ChunkFile.cpp | 65 ++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/Common/ChunkFile.cpp b/Common/ChunkFile.cpp index 5f380347fd..3560c6eeb4 100644 --- a/Common/ChunkFile.cpp +++ b/Common/ChunkFile.cpp @@ -279,18 +279,25 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, } // Make sure we can allocate a buffer to compress before compressing. - size_t comp_len = snappy_max_compressed_length(sz); - u8 *compressed_buffer = (u8 *)malloc(comp_len); + size_t write_len = snappy_max_compressed_length(sz); + u8 *compressed_buffer = (u8 *)malloc(write_len); + u8 *write_buffer = buffer; if (!compressed_buffer) { ERROR_LOG(SAVESTATE, "ChunkReader: Unable to allocate compressed buffer"); // We'll save uncompressed. Better than not saving... + write_len = sz; + } else { + snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &write_len); + free(buffer); + + write_buffer = compressed_buffer; } // Create header SChunkHeader header; header.Compress = compressed_buffer ? 1 : 0; header.Revision = REVISION_CURRENT; - header.ExpectedSize = (u32)sz; + header.ExpectedSize = (u32)write_len; header.UncompressedSize = (u32)sz; truncate_cpy(header.GitVersion, gitVersion); @@ -298,40 +305,26 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, char titleFixed[128]; truncate_cpy(titleFixed, title.c_str()); - // Write to file - if (compressed_buffer) { - snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &comp_len); - free(buffer); - header.ExpectedSize = (u32)comp_len; - - if (!pFile.WriteArray(&header, 1)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - return ERROR_BAD_FILE; - } - if (!pFile.WriteArray(titleFixed, sizeof(titleFixed))) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); - return ERROR_BAD_FILE; - } - if (!pFile.WriteBytes(compressed_buffer, comp_len)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); - return ERROR_BAD_FILE; - } else { - INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)comp_len); - } - free(compressed_buffer); - } else { - if (!pFile.WriteArray(&header, 1)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - free(buffer); - return ERROR_BAD_FILE; - } - if (!pFile.WriteBytes(&buffer[0], sz)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing data"); - free(buffer); - return ERROR_BAD_FILE; - } - free(buffer); + // Now let's start writing out the file... + if (!pFile.WriteArray(&header, 1)) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); + free(write_buffer); + return ERROR_BAD_FILE; } + if (!pFile.WriteArray(titleFixed, sizeof(titleFixed))) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); + free(write_buffer); + return ERROR_BAD_FILE; + } + + if (!pFile.WriteBytes(write_buffer, write_len)) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); + free(write_buffer); + return ERROR_BAD_FILE; + } else if (sz != write_len) { + INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)write_len); + } + free(write_buffer); INFO_LOG(SAVESTATE, "ChunkReader: Done writing %s", filename.c_str()); return ERROR_NONE;