diff options
| -rw-r--r-- | core/fxcrt/fx_memory.cpp | 16 | ||||
| -rw-r--r-- | core/fxcrt/fx_memory.h | 45 | ||||
| -rw-r--r-- | samples/DEPS | 1 | ||||
| -rw-r--r-- | samples/image_diff.cc | 3 | ||||
| -rw-r--r-- | xfa/fgas/font/cfgas_fontmgr.cpp | 9 |
5 files changed, 61 insertions, 13 deletions
diff --git a/core/fxcrt/fx_memory.cpp b/core/fxcrt/fx_memory.cpp index 2afcbcecee..589a4cf508 100644 --- a/core/fxcrt/fx_memory.cpp +++ b/core/fxcrt/fx_memory.cpp @@ -5,10 +5,12 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com #include "core/fxcrt/fx_memory.h" +#include "core/fxcrt/fx_safe_types.h" #include <stdlib.h> // For abort(). pdfium::base::PartitionAllocatorGeneric gArrayBufferPartitionAllocator; +pdfium::base::PartitionAllocatorGeneric gGeneralPartitionAllocator; pdfium::base::PartitionAllocatorGeneric gStringPartitionAllocator; void FXMEM_InitializePartitionAlloc() { @@ -16,25 +18,31 @@ void FXMEM_InitializePartitionAlloc() { if (!s_gPartitionAllocatorsInitialized) { pdfium::base::PartitionAllocGlobalInit(FX_OutOfMemoryTerminate); gArrayBufferPartitionAllocator.init(); + gGeneralPartitionAllocator.init(); gStringPartitionAllocator.init(); s_gPartitionAllocatorsInitialized = true; } } +// TODO(palmer): Remove the |flags| argument. void* FXMEM_DefaultAlloc(size_t byte_size, int flags) { - return (void*)malloc(byte_size); + return pdfium::base::PartitionAllocGeneric(gGeneralPartitionAllocator.root(), + byte_size, "GeneralPartition"); } void* FXMEM_DefaultCalloc(size_t num_elems, size_t byte_size) { - return calloc(num_elems, byte_size); + return FX_SafeAlloc(num_elems, byte_size); } +// TODO(palmer): Remove the |flags| argument. void* FXMEM_DefaultRealloc(void* pointer, size_t new_size, int flags) { - return realloc(pointer, new_size); + return pdfium::base::PartitionReallocGeneric( + gGeneralPartitionAllocator.root(), pointer, new_size, "GeneralPartition"); } +// TODO(palmer): Remove the |flags| argument. void FXMEM_DefaultFree(void* pointer, int flags) { - free(pointer); + pdfium::base::PartitionFree(pointer); } NEVER_INLINE void FX_OutOfMemoryTerminate() { diff --git a/core/fxcrt/fx_memory.h b/core/fxcrt/fx_memory.h index 9a7de4cc2b..a18d74bb23 100644 --- a/core/fxcrt/fx_memory.h +++ b/core/fxcrt/fx_memory.h @@ -27,24 +27,43 @@ void FXMEM_DefaultFree(void* pointer, int flags); #include <memory> #include <new> +#include "core/fxcrt/fx_safe_types.h" #include "third_party/base/allocator/partition_allocator/partition_alloc.h" extern pdfium::base::PartitionAllocatorGeneric gArrayBufferPartitionAllocator; +extern pdfium::base::PartitionAllocatorGeneric gGeneralPartitionAllocator; extern pdfium::base::PartitionAllocatorGeneric gStringPartitionAllocator; void FXMEM_InitializePartitionAlloc(); NEVER_INLINE void FX_OutOfMemoryTerminate(); +inline void* FX_SafeAlloc(size_t num_members, size_t member_size) { + FX_SAFE_SIZE_T total = member_size; + total *= num_members; + if (!total.IsValid()) { + return nullptr; + } + void* result = pdfium::base::PartitionAllocGeneric( + gGeneralPartitionAllocator.root(), total.ValueOrDie(), + "GeneralPartition"); + memset(result, 0, total.ValueOrDie()); + return result; +} + inline void* FX_SafeRealloc(void* ptr, size_t num_members, size_t member_size) { - if (num_members < std::numeric_limits<size_t>::max() / member_size) { - return realloc(ptr, num_members * member_size); + FX_SAFE_SIZE_T size = num_members; + size *= member_size; + if (!size.IsValid()) { + return nullptr; } - return nullptr; + return pdfium::base::PartitionReallocGeneric( + gGeneralPartitionAllocator.root(), ptr, size.ValueOrDie(), + "GeneralPartition"); } inline void* FX_AllocOrDie(size_t num_members, size_t member_size) { // TODO(tsepez): See if we can avoid the implicit memset(0). - if (void* result = calloc(num_members, member_size)) { + if (void* result = FX_SafeAlloc(num_members, member_size)) { return result; } FX_OutOfMemoryTerminate(); // Never returns. @@ -69,18 +88,30 @@ inline void* FX_ReallocOrDie(void* ptr, return nullptr; // Suppress compiler warning. } -// Never returns nullptr. +// These never return nullptr. #define FX_Alloc(type, size) (type*)FX_AllocOrDie(size, sizeof(type)) #define FX_Alloc2D(type, w, h) (type*)FX_AllocOrDie2D(w, h, sizeof(type)) #define FX_Realloc(type, ptr, size) \ (type*)FX_ReallocOrDie(ptr, size, sizeof(type)) // May return nullptr. -#define FX_TryAlloc(type, size) (type*)calloc(size, sizeof(type)) +#define FX_TryAlloc(type, size) (type*)FX_SafeAlloc(size, sizeof(type)) #define FX_TryRealloc(type, ptr, size) \ (type*)FX_SafeRealloc(ptr, size, sizeof(type)) -#define FX_Free(ptr) free(ptr) +inline void FX_Free(void* ptr) { + // TODO(palmer): Removing this check exposes crashes when PDFium callers + // attempt to free |nullptr|. Although libc's |free| allows freeing |NULL|, no + // other Partition Alloc callers need this tolerant behavior. Additionally, + // checking for |nullptr| adds a branch to |PartitionFree|, and it's nice to + // not have to have that. + // + // So this check is hiding (what I consider to be) bugs, and we should try to + // fix them. https://bugs.chromium.org/p/pdfium/issues/detail?id=690 + if (ptr) { + pdfium::base::PartitionFree(ptr); + } +} // The FX_ArraySize(arr) macro returns the # of elements in an array arr. // The expression is a compile-time constant, and therefore can be diff --git a/samples/DEPS b/samples/DEPS index fb322dee20..0a426d6e9c 100644 --- a/samples/DEPS +++ b/samples/DEPS @@ -5,4 +5,5 @@ include_rules = [ '+third_party/zlib', '+v8', '+core/fdrm/crypto/fx_crypt.h', + '+core/fxcrt/fx_memory.h', ] diff --git a/samples/image_diff.cc b/samples/image_diff.cc index 70c74c951c..21460f5a62 100644 --- a/samples/image_diff.cc +++ b/samples/image_diff.cc @@ -19,6 +19,7 @@ #include <string> #include <vector> +#include "core/fxcrt/fx_memory.h" #include "samples/image_diff_png.h" #include "third_party/base/logging.h" #include "third_party/base/numerics/safe_conversions.h" @@ -312,6 +313,8 @@ int DiffImages(const std::string& file1, } int main(int argc, const char* argv[]) { + FXMEM_InitializePartitionAlloc(); + bool histograms = false; bool produce_diff_image = false; std::string filename1; diff --git a/xfa/fgas/font/cfgas_fontmgr.cpp b/xfa/fgas/font/cfgas_fontmgr.cpp index 8430d4f834..6f86c7ca07 100644 --- a/xfa/fgas/font/cfgas_fontmgr.cpp +++ b/xfa/fgas/font/cfgas_fontmgr.cpp @@ -940,7 +940,12 @@ FXFT_Face CFGAS_FontMgr::LoadFace( if (!library) return nullptr; - FXFT_Stream ftStream = FX_Alloc(FXFT_StreamRec, 1); + // TODO(palmer): This memory will be freed with |ft_free| (which is |free|). + // Ultimately, we want to change this to: + // FXFT_Stream ftStream = FX_Alloc(FXFT_StreamRec, 1); + // https://bugs.chromium.org/p/pdfium/issues/detail?id=690 + FXFT_Stream ftStream = + static_cast<FXFT_Stream>(ft_scalloc(sizeof(FXFT_StreamRec), 1)); memset(ftStream, 0, sizeof(FXFT_StreamRec)); ftStream->base = nullptr; ftStream->descriptor.pointer = static_cast<void*>(pFontStream.Get()); @@ -956,7 +961,7 @@ FXFT_Face CFGAS_FontMgr::LoadFace( FXFT_Face pFace = nullptr; if (FXFT_Open_Face(library, &ftArgs, iFaceIndex, &pFace)) { - FX_Free(ftStream); + ft_sfree(ftStream); return nullptr; } |