Abort on OOM by default in FX_Alloc().
authorTom Sepez <tsepez@chromium.org>
Fri, 15 May 2015 22:07:20 +0000 (15:07 -0700)
committerTom Sepez <tsepez@chromium.org>
Fri, 15 May 2015 22:07:20 +0000 (15:07 -0700)
Add a FX_TryAlloc() for those few cases where we might need to continue
in face of OOM.

Remove FX_AllocNL() (the context of its use would suggest that NL
means "No Limit"). This is used for some big allocations, so replace
it with TryAlloc().  Large allocations may be worth trying to continue
from, since there are few and they have a large chance of failing.

R=thestig@chromium.org

Review URL: https://codereview.chromium.org/1128043009

BUILD.gn
core/include/fxcrt/fx_memory.h
core/include/fxcrt/fx_system.h
core/src/fxcodec/codec/fx_codec.cpp
core/src/fxcrt/fx_basic_memmgr.cpp
core/src/fxcrt/fx_basic_memmgr_unittest.cpp [new file with mode: 0644]
core/src/fxge/dib/fx_dib_convert.cpp
core/src/fxge/dib/fx_dib_engine.cpp
core/src/fxge/dib/fx_dib_main.cpp
pdfium.gyp

index ba262de..00774dd 100644 (file)
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -853,6 +853,7 @@ test("pdfium_unittests") {
   sources = [
     "core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp",
     "core/src/fxcrt/fx_basic_bstring_unittest.cpp",
+    "core/src/fxcrt/fx_basic_memmgr_unittest.cpp",
     "core/src/fxcrt/fx_basic_wstring_unittest.cpp",
     "testing/fx_string_testhelpers.cpp",
     "testing/fx_string_testhelpers.h",
index 25d82f6..afce911 100644 (file)
 #include "fx_system.h"
 
 #ifdef __cplusplus
-#include <new>
 extern "C" {
 #endif
-#define FX_Alloc(type, size)                                           (type*)calloc(size, sizeof(type))
-#define FX_Realloc(type, ptr, size)                                    (type*)realloc(ptr, sizeof(type) * (size))
-#define FX_AllocNL(type, size)                                         FX_Alloc(type, size)
-#define FX_ReallocNL(type, ptr, size)                          FX_Realloc(type, ptr, size)
-#define FX_Free(ptr)                                                           free(ptr)
+// For external C libraries to malloc through PDFium. These may return NULL.
 void*  FXMEM_DefaultAlloc(size_t byte_size, int flags);
 void*  FXMEM_DefaultRealloc(void* pointer, size_t new_size, int flags);
 void   FXMEM_DefaultFree(void* pointer, int flags);
 #ifdef __cplusplus
+}  // extern "C"
+
+#include <stdlib.h>
+#include <limits>
+#include <new>
+
+NEVER_INLINE void FX_OutOfMemoryTerminate();
+
+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);
+    }
+    return nullptr;
+}
+
+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)) {
+        return result;
+    }
+    FX_OutOfMemoryTerminate();  // Never returns.
+    return nullptr;  // Suppress compiler warning.
 }
 
+inline void* FX_ReallocOrDie(void* ptr, size_t num_members, size_t member_size) {
+    if (void* result = FX_SafeRealloc(ptr, num_members, member_size)) {
+        return result;
+    }
+    FX_OutOfMemoryTerminate();  // Never returns.
+    return nullptr;  // Suppress compiler warning.
+}
+
+// Never returns NULL.
+#define FX_Alloc(type, size) (type*)FX_AllocOrDie(size, sizeof(type))
+#define FX_Realloc(type, ptr, size) \
+    (type*)FX_ReallocOrDie(ptr, size, sizeof(type))
+
+// May return NULL.
+#define FX_TryAlloc(type, size) (type*)calloc(size, sizeof(type))
+#define FX_TryRealloc(type, ptr, size) \
+    (type*)FX_SafeRealloc(ptr, size, sizeof(type))
+
+#define FX_Free(ptr) free(ptr)
+
 class CFX_DestructObject 
 {
 public:
@@ -69,5 +106,5 @@ private:
 
     void*      m_pFirstTrunk;
 };
-#endif
-#endif
+#endif  // __cplusplust
+#endif  // _FX_MEMORY_H_
index 9cc165f..96030ca 100644 (file)
@@ -6,10 +6,12 @@
 
 #ifndef _FX_SYSTEM_H_
 #define _FX_SYSTEM_H_
+
 #define _FX_WIN32_DESKTOP_             1
 #define _FX_LINUX_DESKTOP_             4
 #define _FX_MACOSX_                            7
 #define _FX_ANDROID_                   12
+
 #define _FXM_PLATFORM_WINDOWS_         1
 #define _FXM_PLATFORM_LINUX_           2
 #define _FXM_PLATFORM_APPLE_           3
@@ -341,12 +343,20 @@ int                       FXSYS_round(FX_FLOAT f);
 #define PRIuS "zu"
 #endif
 
-#else  // _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
+#else  // _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
 
 #if !defined(PRIuS)
 #define PRIuS "Iu"
 #endif
 
-#endif
+#endif  // _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
 
-#endif
+// Prevent a function from ever being inlined, typically because we'd
+// like it to appear in stack traces.
+#if  _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
+#define NEVER_INLINE __declspec(noinline)
+#else  // _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
+#define NEVER_INLINE __attribute__((__noinline__))
+#endif  // _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
+
+#endif  // _FX_SYSTEM_H_
index d4df196..e8aad71 100644 (file)
@@ -119,7 +119,7 @@ void CCodec_ScanlineDecoder::DownScale(int dest_width, int dest_height)
         FX_Free(m_pDataCache);
         m_pDataCache = NULL;
     }
-    m_pDataCache = (CCodec_ImageDataCache*)FX_AllocNL(FX_BYTE, sizeof(CCodec_ImageDataCache) + m_Pitch * m_OutputHeight);
+    m_pDataCache = (CCodec_ImageDataCache*)FX_TryAlloc(FX_BYTE, sizeof(CCodec_ImageDataCache) + m_Pitch * m_OutputHeight);
     if (m_pDataCache == NULL) {
         return;
     }
index 3b3211c..63c609d 100644 (file)
@@ -4,10 +4,9 @@
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
-#include "../../include/fxcrt/fx_basic.h"
-#ifdef __cplusplus
-extern "C" {
-#endif
+#include <stdlib.h>  // For abort().
+#include "../../include/fxcrt/fx_memory.h"
+
 void*  FXMEM_DefaultAlloc(size_t byte_size, int flags)
 {
     return (void*)malloc(byte_size);
@@ -20,9 +19,13 @@ void FXMEM_DefaultFree(void* pointer, int flags)
 {
     free(pointer);
 }
-#ifdef __cplusplus
+
+NEVER_INLINE void FX_OutOfMemoryTerminate() {
+    // Termimate cleanly if we can, else crash at a specific address (0xbd).
+    abort();
+    reinterpret_cast<void(*)()>(0xbd)();
 }
-#endif
+
 CFX_GrowOnlyPool::CFX_GrowOnlyPool(size_t trunk_size)
 {
     m_TrunkSize = trunk_size;
diff --git a/core/src/fxcrt/fx_basic_memmgr_unittest.cpp b/core/src/fxcrt/fx_basic_memmgr_unittest.cpp
new file mode 100644 (file)
index 0000000..20fd93d
--- /dev/null
@@ -0,0 +1,47 @@
+// Copyright 2015 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <limits>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "../../include/fxcrt/fx_memory.h"
+
+namespace {
+
+const size_t kMaxByteAlloc = std::numeric_limits<size_t>::max();
+const size_t kMaxIntAlloc = kMaxByteAlloc / sizeof(int);
+const size_t kOverflowIntAlloc = kMaxIntAlloc + 100;
+
+}  // namespace
+
+TEST(fxcrt, FX_AllocOOM) {
+    EXPECT_DEATH_IF_SUPPORTED(FX_Alloc(int, kMaxIntAlloc), "");
+    EXPECT_DEATH_IF_SUPPORTED(FX_Alloc(int, kOverflowIntAlloc), "");
+
+    int* ptr = FX_Alloc(int, 1);
+    EXPECT_TRUE(ptr);
+    EXPECT_DEATH_IF_SUPPORTED(FX_Realloc(int, ptr, kMaxIntAlloc), "");
+    EXPECT_DEATH_IF_SUPPORTED(FX_Realloc(int, ptr, kOverflowIntAlloc), "");
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, FX_TryAllocOOM) {
+    EXPECT_FALSE(FX_TryAlloc(int, kMaxIntAlloc));
+    EXPECT_FALSE(FX_TryAlloc(int, kOverflowIntAlloc));
+
+    int* ptr = FX_Alloc(int, 1);
+    EXPECT_TRUE(ptr);
+    EXPECT_FALSE(FX_TryRealloc(int, ptr, kMaxIntAlloc));
+    EXPECT_FALSE(FX_TryRealloc(int, ptr, kOverflowIntAlloc));
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, FXMEM_DefaultOOM) {
+    EXPECT_FALSE(FXMEM_DefaultAlloc(kMaxByteAlloc, 0));
+
+    void* ptr = FXMEM_DefaultAlloc(1, 0);
+    EXPECT_TRUE(ptr);
+    EXPECT_FALSE(FXMEM_DefaultRealloc(ptr, kMaxByteAlloc, 0));
+    FXMEM_DefaultFree(ptr, 0);
+}
index 5b85c7d..4a8befe 100644 (file)
@@ -1008,7 +1008,7 @@ FX_BOOL CFX_DIBitmap::ConvertFormat(FXDIB_Format dest_format, void* pIccTransfor
     }
     int dest_bpp = dest_format & 0xff;
     int dest_pitch = (dest_bpp * m_Width + 31) / 32 * 4;
-    FX_LPBYTE dest_buf = FX_AllocNL(FX_BYTE, dest_pitch * m_Height + 4);
+    FX_LPBYTE dest_buf = FX_TryAlloc(FX_BYTE, dest_pitch * m_Height + 4);
     if (dest_buf == NULL) {
         return FALSE;
     }
index 997ca1d..5053c30 100644 (file)
@@ -28,7 +28,7 @@ void CWeightTable::Calc(int dest_len, int dest_min, int dest_max, int src_len, i
     if ((dest_max - dest_min) > (int)((1U << 30) - 4) / m_ItemSize) {
         return;
     }
-    m_pWeightTables = FX_AllocNL(FX_BYTE, (dest_max - dest_min) * m_ItemSize + 4);
+    m_pWeightTables = FX_TryAlloc(FX_BYTE, (dest_max - dest_min) * m_ItemSize + 4);
     if (m_pWeightTables == NULL) {
         return;
     }
@@ -202,7 +202,7 @@ CStretchEngine::CStretchEngine(IFX_ScanlineComposer* pDestBitmap, FXDIB_Format d
     }
     size += 31;
     size = size / 32 * 4;
-    m_pDestScanline = FX_AllocNL(FX_BYTE, size);
+    m_pDestScanline = FX_TryAlloc(FX_BYTE, size);
     if (m_pDestScanline == NULL) {
         return;
     }
@@ -311,7 +311,7 @@ FX_BOOL CStretchEngine::StartStretchHorz()
     if (m_DestWidth == 0 || m_pDestScanline == NULL || m_SrcClip.Height() > (int)((1U << 29) / m_InterPitch) || m_SrcClip.Height() == 0) {
         return FALSE;
     }
-    m_pInterBuf = FX_AllocNL(unsigned char, m_SrcClip.Height() * m_InterPitch);
+    m_pInterBuf = FX_TryAlloc(unsigned char, m_SrcClip.Height() * m_InterPitch);
     if (m_pInterBuf == NULL) {
         return FALSE;
     }
@@ -321,7 +321,7 @@ FX_BOOL CStretchEngine::StartStretchHorz()
             return FALSE;
         }
         FX_DWORD size = (m_DestClip.Width() * 8 + 31) / 32 * 4;
-        m_pDestMaskScanline = FX_AllocNL(unsigned char, size);
+        m_pDestMaskScanline = FX_TryAlloc(unsigned char, size);
         if (!m_pDestMaskScanline) {
             return FALSE;
         }
index 59c0bd6..a54c9ab 100644 (file)
@@ -84,7 +84,7 @@ FX_BOOL CFX_DIBitmap::Create(int width, int height, FXDIB_Format format, FX_LPBY
         int size = pitch * height + 4;
         int oomlimit = _MAX_OOM_LIMIT_;
         if (oomlimit >= 0 && size >= oomlimit) {
-            m_pBuffer = FX_AllocNL(FX_BYTE, size);
+            m_pBuffer = FX_TryAlloc(FX_BYTE, size);
         } else {
             m_pBuffer = FX_Alloc(FX_BYTE, size);
         }
index d8df460..7f1d908 100644 (file)
       'sources': [
         'core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp',
         'core/src/fxcrt/fx_basic_bstring_unittest.cpp',
+        'core/src/fxcrt/fx_basic_memmgr_unittest.cpp',
         'core/src/fxcrt/fx_basic_wstring_unittest.cpp',
         'testing/fx_string_testhelpers.h',
         'testing/fx_string_testhelpers.cpp',