summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChris Palmer <palmer@chromium.org>2017-06-22 10:10:17 -0700
committerChromium commit bot <commit-bot@chromium.org>2017-06-22 17:41:45 +0000
commit7726a581626bbb72d6ab294ae1adbad4ca10dfb0 (patch)
treeb9ab95fb40d0f05ef1cc2e3010d5ee75830714f1
parent9a25eded6f4afe3f4815a00b34b31d3a0fbadb69 (diff)
downloadpdfium-chromium/3140.tar.xz
Add a |gGeneralPartitionAllocator| and use it in the |FX_*Alloc| wrappers.chromium/3142chromium/3141chromium/3140chromium/3139
BUG=pdfium:690 Change-Id: I922279e4bdc8b411f47f49798155e8f9118c1396 Reviewed-on: https://pdfium-review.googlesource.com/6552 Commit-Queue: Chris Palmer <palmer@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org>
-rw-r--r--core/fxcrt/fx_memory.cpp16
-rw-r--r--core/fxcrt/fx_memory.h45
-rw-r--r--samples/DEPS1
-rw-r--r--samples/image_diff.cc3
-rw-r--r--xfa/fgas/font/cfgas_fontmgr.cpp9
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;
}