Merge to XFA: Abort on OOM by default in FX_Alloc().
authorTom Sepez <tsepez@chromium.org>
Fri, 15 May 2015 23:30:52 +0000 (16:30 -0700)
committerTom Sepez <tsepez@chromium.org>
Fri, 15 May 2015 23:30:52 +0000 (16:30 -0700)
Original Review URL: https://codereview.chromium.org/1128043009
Original Review URL: https://codereview.chromium.org/1142463005

R=thestig@chromium.org
TBR=thestig@chromium.org

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

12 files changed:
BUILD.gn
core/include/fxcrt/fx_memory.h
core/include/fxcrt/fx_system.h
core/src/fxcodec/codec/fx_codec.cpp
core/src/fxcodec/lbmp/fx_bmp.cpp
core/src/fxcodec/lgif/fx_gif.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 b9be538..1dedaae 100644 (file)
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1556,6 +1556,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 e6685ef..c1c0740 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>
 
 #define FX_NEW 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:
@@ -71,5 +108,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 36dff5e..0c30684 100644 (file)
@@ -123,7 +123,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 4c6eb61..e868fde 100644 (file)
@@ -882,7 +882,7 @@ FX_BOOL _bmp_encode_image( bmp_compress_struct_p bmp_ptr, FX_LPBYTE& dst_buf, FX
         pal_size = sizeof(FX_DWORD) * bmp_ptr->info_header.biClrUsed;\r
     }\r
     dst_size = head_size + sizeof(FX_DWORD) * bmp_ptr->pal_num;\r
-    dst_buf = FX_AllocNL(FX_BYTE, dst_size);\r
+    dst_buf = FX_TryAlloc(FX_BYTE, dst_size);\r
     if (dst_buf == NULL) {\r
         return FALSE;\r
     }\r
index 08c1803..bc6109e 100644 (file)
@@ -1147,7 +1147,7 @@ static FX_BOOL _gif_write_header( gif_compress_struct_p gif_ptr, FX_LPBYTE& dst_
         return TRUE;\r
     }\r
     dst_len = sizeof(GifHeader) + sizeof(GifLSD) + sizeof(GifGF);\r
-    dst_buf = FX_AllocNL(FX_BYTE, dst_len);\r
+    dst_buf = FX_TryAlloc(FX_BYTE, dst_len);\r
     if (dst_buf == NULL)       {\r
         return FALSE;\r
     }\r
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..565021d
--- /dev/null
@@ -0,0 +1,63 @@
+// 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
+
+// TODO(tsepez): re-enable OOM tests if we can find a way to
+// prevent it from hosing the bots.
+TEST(fxcrt, DISABLED_FX_AllocOOM) {
+    EXPECT_DEATH_IF_SUPPORTED(FX_Alloc(int, kMaxIntAlloc), "");
+
+    int* ptr = FX_Alloc(int, 1);
+    EXPECT_TRUE(ptr);
+    EXPECT_DEATH_IF_SUPPORTED(FX_Realloc(int, ptr, kMaxIntAlloc), "");
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, FX_AllocOverflow) {
+    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, kOverflowIntAlloc), "");
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, DISABLED_FX_TryAllocOOM) {
+    EXPECT_FALSE(FX_TryAlloc(int, kMaxIntAlloc));
+
+    int* ptr = FX_Alloc(int, 1);
+    EXPECT_TRUE(ptr);
+    EXPECT_FALSE(FX_TryRealloc(int, ptr, kMaxIntAlloc));
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, FX_TryAllocOverflow) {
+    EXPECT_FALSE(FX_TryAlloc(int, kOverflowIntAlloc));
+
+    int* ptr = FX_Alloc(int, 1);
+    EXPECT_TRUE(ptr);
+    EXPECT_FALSE(FX_TryRealloc(int, ptr, kOverflowIntAlloc));
+    FX_Free(ptr);
+}
+
+TEST(fxcrt, DISABLED_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 cfcbc70..69432fd 100644 (file)
@@ -1015,7 +1015,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 f4c0ef1..4d398d8 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 2cb41f6..d9e4018 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 7d712c6..8cd3c37 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',