From 2c697754a9d96855c30d1809e67cc676c59de7d1 Mon Sep 17 00:00:00 2001 From: Tim Rowley Date: Fri, 28 Oct 2016 17:21:01 -0500 Subject: [PATCH] swr: [rasterizer archrast] fix double free issue Reviewed-by: Bruce Cherniak --- .../swr/rasterizer/archrast/archrast.cpp | 10 +++++----- .../swr/rasterizer/archrast/archrast.h | 2 +- .../swr/rasterizer/archrast/eventmanager.h | 20 +++++++++++++++---- .../drivers/swr/rasterizer/core/api.cpp | 1 - .../drivers/swr/rasterizer/core/context.h | 6 +++--- .../scripts/templates/ar_event_cpp.template | 4 ++-- .../scripts/templates/ar_event_h.template | 7 +++++-- .../templates/ar_eventhandler_h.template | 5 ++++- .../templates/ar_eventhandlerfile_h.template | 10 +++++----- 9 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp b/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp index c29bb884588..574098d80a8 100644 --- a/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp +++ b/src/gallium/drivers/swr/rasterizer/archrast/archrast.cpp @@ -43,8 +43,8 @@ namespace ArchRast EventHandlerStatsFile(uint32_t id) : EventHandlerFile(id) {} // These are events that we're not interested in saving in stats event files. - virtual void handle(Start& event) {} - virtual void handle(End& event) {} + virtual void Handle(Start& event) {} + virtual void Handle(End& event) {} }; static EventManager* FromHandle(HANDLE hThreadContext) @@ -64,7 +64,7 @@ namespace ArchRast if (pManager && pHandler) { - pManager->attach(pHandler); + pManager->Attach(pHandler); return pManager; } @@ -82,11 +82,11 @@ namespace ArchRast } // Dispatch event for this thread. - void dispatch(HANDLE hThreadContext, Event& event) + void Dispatch(HANDLE hThreadContext, Event& event) { EventManager* pManager = FromHandle(hThreadContext); SWR_ASSERT(pManager != nullptr); - pManager->dispatch(event); + pManager->Dispatch(event); } } diff --git a/src/gallium/drivers/swr/rasterizer/archrast/archrast.h b/src/gallium/drivers/swr/rasterizer/archrast/archrast.h index bdb3afbed99..e6376f208fe 100644 --- a/src/gallium/drivers/swr/rasterizer/archrast/archrast.h +++ b/src/gallium/drivers/swr/rasterizer/archrast/archrast.h @@ -36,6 +36,6 @@ namespace ArchRast void DestroyThreadContext(HANDLE hThreadContext); // Dispatch event for this thread. - void dispatch(HANDLE hThreadContext, Event& event); + void Dispatch(HANDLE hThreadContext, Event& event); }; diff --git a/src/gallium/drivers/swr/rasterizer/archrast/eventmanager.h b/src/gallium/drivers/swr/rasterizer/archrast/eventmanager.h index d162ca8fe57..78ba8f3e2d7 100644 --- a/src/gallium/drivers/swr/rasterizer/archrast/eventmanager.h +++ b/src/gallium/drivers/swr/rasterizer/archrast/eventmanager.h @@ -43,24 +43,36 @@ namespace ArchRast class EventManager { public: - void attach(EventHandler* pHandler) + EventManager() {} + + ~EventManager() + { + // Event manager owns destroying handler objects once attached. + ///@note See comment for Detach. + for (auto pHandler : mHandlers) + { + delete pHandler; + } + } + + void Attach(EventHandler* pHandler) { mHandlers.push_back(pHandler); } - void dispatch(Event& event) + void Dispatch(Event& event) { ///@todo Add event filter check here. for (auto pHandler : mHandlers) { - event.accept(pHandler); + event.Accept(pHandler); } } private: // Handlers stay registered for life - void detach(EventHandler* pHandler) { SWR_ASSERT(0); } + void Detach(EventHandler* pHandler) { SWR_ASSERT(0); } std::vector mHandlers; }; diff --git a/src/gallium/drivers/swr/rasterizer/core/api.cpp b/src/gallium/drivers/swr/rasterizer/core/api.cpp index e8e1fdd4841..542eccda95e 100644 --- a/src/gallium/drivers/swr/rasterizer/core/api.cpp +++ b/src/gallium/drivers/swr/rasterizer/core/api.cpp @@ -394,7 +394,6 @@ void SwrDestroyContext(HANDLE hContext) } delete[] pContext->ppScratch; - delete[] pContext->pArContext; delete[] pContext->pStats; delete(pContext->pHotTileMgr); diff --git a/src/gallium/drivers/swr/rasterizer/core/context.h b/src/gallium/drivers/swr/rasterizer/core/context.h index 23685b42bb2..e7c462da8b9 100644 --- a/src/gallium/drivers/swr/rasterizer/core/context.h +++ b/src/gallium/drivers/swr/rasterizer/core/context.h @@ -523,9 +523,9 @@ struct SWR_CONTEXT #define AR_API_CTX pContext->pArContext[pContext->NumWorkerThreads] #ifdef KNOB_ENABLE_AR - #define _AR_BEGIN(ctx, type, id) ArchRast::dispatch(ctx, ArchRast::Start(ArchRast::type, id)) - #define _AR_END(ctx, type, count) ArchRast::dispatch(ctx, ArchRast::End(ArchRast::type, count)) - #define _AR_EVENT(ctx, event) ArchRast::dispatch(ctx, ArchRast::event) + #define _AR_BEGIN(ctx, type, id) ArchRast::Dispatch(ctx, ArchRast::Start(ArchRast::type, id)) + #define _AR_END(ctx, type, count) ArchRast::Dispatch(ctx, ArchRast::End(ArchRast::type, count)) + #define _AR_EVENT(ctx, event) ArchRast::Dispatch(ctx, ArchRast::event) #else #ifdef KNOB_ENABLE_RDTSC #define _AR_BEGIN(ctx, type, id) (void)ctx; RDTSC_START(type) diff --git a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_cpp.template b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_cpp.template index d033d93e734..4b3dcd57dbc 100644 --- a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_cpp.template +++ b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_cpp.template @@ -34,8 +34,8 @@ using namespace ArchRast; % for name in protos['event_names']: -void ${name}::accept(EventHandler* pHandler) +void ${name}::Accept(EventHandler* pHandler) { - pHandler->handle(*this); + pHandler->Handle(*this); } % endfor diff --git a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_h.template b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_h.template index b0e67961304..68926ea8053 100644 --- a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_h.template +++ b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_event_h.template @@ -51,7 +51,10 @@ namespace ArchRast ////////////////////////////////////////////////////////////////////////// struct Event { - virtual void accept(EventHandler* pHandler) = 0; + Event() {} + virtual ~Event() {} + + virtual void Accept(EventHandler* pHandler) = 0; }; % for name in protos['event_names']: @@ -96,7 +99,7 @@ namespace ArchRast % endfor } - virtual void accept(EventHandler* pHandler); + virtual void Accept(EventHandler* pHandler); }; % endfor } \ No newline at end of file diff --git a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandler_h.template b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandler_h.template index 53e5a191b0b..95c54426b50 100644 --- a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandler_h.template +++ b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandler_h.template @@ -39,8 +39,11 @@ namespace ArchRast class EventHandler { public: + EventHandler() {} + virtual ~EventHandler() {} + % for name in protos['event_names']: - virtual void handle(${name}& event) {} + virtual void Handle(${name}& event) {} % endfor }; } diff --git a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandlerfile_h.template b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandlerfile_h.template index 5310bf55b34..0a651a4e4a5 100644 --- a/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandlerfile_h.template +++ b/src/gallium/drivers/swr/rasterizer/scripts/templates/ar_eventhandlerfile_h.template @@ -69,12 +69,12 @@ namespace ArchRast #endif } - ~EventHandlerFile() + virtual ~EventHandlerFile() { if (mFile.is_open()) mFile.close(); } - void write(uint32_t eventId, const char* pBlock, uint32_t size) + void Write(uint32_t eventId, const char* pBlock, uint32_t size) { if (!mFile.is_open()) { @@ -86,12 +86,12 @@ namespace ArchRast } % for name in protos['event_names']: - virtual void handle(${name}& event) + virtual void Handle(${name}& event) { % if protos['events'][name]['num_fields'] == 0: - write(${protos['events'][name]['event_id']}, (char*)&event.data, 0); + Write(${protos['events'][name]['event_id']}, (char*)&event.data, 0); % else: - write(${protos['events'][name]['event_id']}, (char*)&event.data, sizeof(event.data)); + Write(${protos['events'][name]['event_id']}, (char*)&event.data, sizeof(event.data)); %endif } % endfor -- 2.30.2