From 012ae898a069bda7afbfdad4eb4c8ba042b68dc7 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 25 Apr 2017 16:39:34 -0700 Subject: Get rid of a few |new|s in CPDF_Document. The chain of destructors may attempt to use m_pDocPage after it has been set to null by the unique_ptr destructor. Verify it is still present before using it from any code that may be called from some other CPDF_ destructor. Change-Id: I007160231d73feed85d90efc687d6da993653f96 Reviewed-on: https://pdfium-review.googlesource.com/4499 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fpdfapi/font/cpdf_font.cpp | 7 +++++-- core/fpdfapi/page/cpdf_color.cpp | 11 +++++++---- core/fpdfapi/page/cpdf_colorspace.cpp | 12 +++++++++--- core/fpdfapi/page/cpdf_shadingpattern.cpp | 7 +++++-- core/fpdfapi/parser/cpdf_document.cpp | 5 ++--- core/fpdfapi/parser/cpdf_document.h | 5 ++--- core/fpdfapi/render/cpdf_dibsource.cpp | 8 +++++--- 7 files changed, 35 insertions(+), 20 deletions(-) diff --git a/core/fpdfapi/font/cpdf_font.cpp b/core/fpdfapi/font/cpdf_font.cpp index b9be3aa868..53186daec6 100644 --- a/core/fpdfapi/font/cpdf_font.cpp +++ b/core/fpdfapi/font/cpdf_font.cpp @@ -61,8 +61,11 @@ CPDF_Font::CPDF_Font() CPDF_Font::~CPDF_Font() { if (m_pFontFile) { - m_pDocument->GetPageData()->MaybePurgeFontFileStreamAcc( - m_pFontFile->GetStream()->AsStream()); + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) { + pPageData->MaybePurgeFontFileStreamAcc( + m_pFontFile->GetStream()->AsStream()); + } } } diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp index f0e433c243..59ce6dab28 100644 --- a/core/fpdfapi/page/cpdf_color.cpp +++ b/core/fpdfapi/page/cpdf_color.cpp @@ -42,10 +42,13 @@ void CPDF_Color::ReleaseBuffer() { } void CPDF_Color::ReleaseColorSpace() { - if (m_pCS && m_pCS->m_pDocument) { - m_pCS->m_pDocument->GetPageData()->ReleaseColorSpace(m_pCS->GetArray()); - m_pCS = nullptr; - } + if (!m_pCS || !m_pCS->m_pDocument) + return; + + auto* pPageData = m_pCS->m_pDocument->GetPageData(); + if (pPageData) + pPageData->ReleaseColorSpace(m_pCS->GetArray()); + m_pCS = nullptr; } void CPDF_Color::SetColorSpace(CPDF_ColorSpace* pCS) { diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp index 400b7a9150..111d45f17e 100644 --- a/core/fpdfapi/page/cpdf_colorspace.cpp +++ b/core/fpdfapi/page/cpdf_colorspace.cpp @@ -852,7 +852,9 @@ CPDF_ICCBasedCS::~CPDF_ICCBasedCS() { if (m_pProfile && m_pDocument) { CPDF_Stream* pStream = m_pProfile->GetStream(); m_pProfile.Reset(); // Give up our reference first. - m_pDocument->GetPageData()->MaybePurgeIccProfile(pStream); + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) + pPageData->MaybePurgeIccProfile(pStream); } } @@ -1049,7 +1051,9 @@ CPDF_IndexedCS::~CPDF_IndexedCS() { FX_Free(m_pCompMinMax); CPDF_ColorSpace* pCS = m_pCountedBaseCS ? m_pCountedBaseCS->get() : nullptr; if (pCS && m_pDocument) { - m_pDocument->GetPageData()->ReleaseColorSpace(pCS->GetArray()); + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) + pPageData->ReleaseColorSpace(pCS->GetArray()); } } @@ -1132,7 +1136,9 @@ CPDF_PatternCS::CPDF_PatternCS(CPDF_Document* pDoc) CPDF_PatternCS::~CPDF_PatternCS() { CPDF_ColorSpace* pCS = m_pCountedBaseCS ? m_pCountedBaseCS->get() : nullptr; if (pCS && m_pDocument) { - m_pDocument->GetPageData()->ReleaseColorSpace(pCS->GetArray()); + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) + pPageData->ReleaseColorSpace(pCS->GetArray()); } } diff --git a/core/fpdfapi/page/cpdf_shadingpattern.cpp b/core/fpdfapi/page/cpdf_shadingpattern.cpp index e19ffaf0c4..133d32ff3a 100644 --- a/core/fpdfapi/page/cpdf_shadingpattern.cpp +++ b/core/fpdfapi/page/cpdf_shadingpattern.cpp @@ -50,8 +50,11 @@ CPDF_ShadingPattern::CPDF_ShadingPattern(CPDF_Document* pDoc, CPDF_ShadingPattern::~CPDF_ShadingPattern() { CPDF_ColorSpace* pCS = m_pCountedCS ? m_pCountedCS->get() : nullptr; - if (pCS && m_pDocument) - m_pDocument->GetPageData()->ReleaseColorSpace(pCS->GetArray()); + if (pCS && m_pDocument) { + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) + pPageData->ReleaseColorSpace(pCS->GetArray()); + } } CPDF_TilingPattern* CPDF_ShadingPattern::AsTilingPattern() { diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp index c23637e8fa..48791262ea 100644 --- a/core/fpdfapi/parser/cpdf_document.cpp +++ b/core/fpdfapi/parser/cpdf_document.cpp @@ -348,14 +348,13 @@ CPDF_Document::CPDF_Document(std::unique_ptr pParser) m_bLinearized(false), m_iFirstPageNo(0), m_dwFirstPageObjNum(0), - m_pDocPage(new CPDF_DocPageData(this)), - m_pDocRender(new CPDF_DocRenderData(this)) { + m_pDocPage(pdfium::MakeUnique(this)), + m_pDocRender(pdfium::MakeUnique(this)) { if (pParser) SetLastObjNum(m_pParser->GetLastObjNum()); } CPDF_Document::~CPDF_Document() { - delete m_pDocPage; CPDF_ModuleMgr::Get()->GetPageModule()->ClearStockFont(this); } diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h index 8d3525419f..2f54ba7c28 100644 --- a/core/fpdfapi/parser/cpdf_document.h +++ b/core/fpdfapi/parser/cpdf_document.h @@ -58,7 +58,7 @@ class CPDF_Document : public CPDF_IndirectObjectHolder { CPDF_Dictionary* GetPage(int iPage); int GetPageIndex(uint32_t objnum); uint32_t GetUserPermissions() const; - CPDF_DocPageData* GetPageData() const { return m_pDocPage; } + CPDF_DocPageData* GetPageData() const { return m_pDocPage.get(); } void SetPageObjNum(int iPage, uint32_t objNum); @@ -142,8 +142,7 @@ class CPDF_Document : public CPDF_IndirectObjectHolder { bool m_bLinearized; int m_iFirstPageNo; uint32_t m_dwFirstPageObjNum; - // TODO(thestig): Figure out why this cannot be a std::unique_ptr. - CPDF_DocPageData* m_pDocPage; + std::unique_ptr m_pDocPage; std::unique_ptr m_pDocRender; std::unique_ptr m_pCodecContext; std::unique_ptr m_pLinksContext; diff --git a/core/fpdfapi/render/cpdf_dibsource.cpp b/core/fpdfapi/render/cpdf_dibsource.cpp index e60d0329d0..3e00c8a23c 100644 --- a/core/fpdfapi/render/cpdf_dibsource.cpp +++ b/core/fpdfapi/render/cpdf_dibsource.cpp @@ -131,9 +131,11 @@ CPDF_DIBSource::~CPDF_DIBSource() { FX_Free(m_pLineBuf); m_pCachedBitmap.Reset(); // TODO(tsepez): determine if required early here. FX_Free(m_pCompData); - CPDF_ColorSpace* pCS = m_pColorSpace; - if (pCS && m_pDocument) - m_pDocument->GetPageData()->ReleaseColorSpace(pCS->GetArray()); + if (m_pColorSpace && m_pDocument) { + auto* pPageData = m_pDocument->GetPageData(); + if (pPageData) + pPageData->ReleaseColorSpace(m_pColorSpace->GetArray()); + } } bool CPDF_DIBSource::Load(CPDF_Document* pDoc, const CPDF_Stream* pStream) { -- cgit v1.2.3