Use __attribute__((destructor)), not atexit(3).

Submitted by Koop Mast on Aug. 27, 2015, 10:44 a.m.

Details

Message ID 1440672283-93639-1-git-send-email-kwm@rainbow-runner.nl
State New
Headers show

Not browsing as part of any series.

Commit Message

Koop Mast Aug. 27, 2015, 10:44 a.m.
On Linux, atexit(3) registered functions are called at program exit or
during module unload. The latter is a Glibc extension not supported by
FreeBSD. This means that, on FreeBSD, the registered function could be
called after the module was unloaded, causing the application to crash.
---
 backend/src/backend/gen_insn_selection.cpp | 2 +-
 backend/src/sys/alloc.cpp                  | 4 ++--
 src/performance.c                          | 7 +------
 utests/utest.cpp                           | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
index b84bb4b..e90fd8d 100644
--- a/backend/src/backend/gen_insn_selection.cpp
+++ b/backend/src/backend/gen_insn_selection.cpp
@@ -1776,11 +1776,11 @@  namespace gbe
 
   // Boiler plate to initialize the selection library at c++ pre-main
   static SelectionLibrary *selLib = NULL;
+  __attribute__((destructor))
   static void destroySelectionLibrary(void) { GBE_DELETE(selLib); }
   static struct SelectionLibraryInitializer {
     SelectionLibraryInitializer(void) {
       selLib = GBE_NEW_NO_ARG(SelectionLibrary);
-      atexit(destroySelectionLibrary);
     }
   } selectionLibraryInitializer;
 
diff --git a/backend/src/sys/alloc.cpp b/backend/src/sys/alloc.cpp
index 08dc7b1..30dc887 100644
--- a/backend/src/sys/alloc.cpp
+++ b/backend/src/sys/alloc.cpp
@@ -140,16 +140,17 @@  namespace gbe
   static bool isMutexInitializing = true;
   static size_t memDebuggerCurrSize(0u);
   static size_t memDebuggerMaxSize(0u);
+  __attribute__((destructor))
   static void SizeMutexDeallocate(void) { if (sizeMutex) delete sizeMutex; }
   static void SizeMutexAllocate(void) {
     if (sizeMutex == NULL && isMutexInitializing == false) {
       isMutexInitializing = true;
       sizeMutex = new MutexSys;
-      atexit(SizeMutexDeallocate);
     }
   }
 
   /*! Stop the memory debugger */
+  __attribute__((destructor))
   static void MemDebuggerEnd(void) {
     MemDebugger *_debug = memDebugger;
     memDebugger = NULL;
@@ -172,7 +173,6 @@  namespace gbe
   /*! Start the memory debugger */
   static void MemDebuggerStart(void) {
     if (memDebugger == NULL) {
-      atexit(MemDebuggerEnd);
       memDebugger = new MemDebugger;
     }
   }
diff --git a/src/performance.c b/src/performance.c
index 85cd481..15acded 100644
--- a/src/performance.c
+++ b/src/performance.c
@@ -37,7 +37,6 @@  typedef struct storage
 
 
 static storage record;
-static int atexit_registered = 0;
 
 
 static context_storage_node * prev_context_pointer = NULL;
@@ -170,6 +169,7 @@  static int cmp(const void *a, const void *b)
     return 0;
 }
 
+__attribute__((destructor))
 static void print_time_info()
 {
   context_storage_node *p_context = record.context_storage;
@@ -273,11 +273,6 @@  static void print_time_info()
 
 static void insert(cl_context context, const char *kernel_name, const char *build_opt, float time)
 {
-  if(!atexit_registered)
-  {
-    atexit_registered = 1;
-    atexit(print_time_info);
-  }
   context_storage_node *p_context = find_context(context);
   kernel_storage_node *p_kernel = find_kernel(p_context, kernel_name, build_opt);
   prev_context_pointer = p_context;
diff --git a/utests/utest.cpp b/utests/utest.cpp
index 0a03d8b..3d6e001 100644
--- a/utests/utest.cpp
+++ b/utests/utest.cpp
@@ -44,6 +44,7 @@  vector<UTest> *UTest::utestList = NULL;
 RStatistics UTest::retStatistics;
 
 void releaseUTestList(void) { delete UTest::utestList; }
+__attribute__((destructor))
 void runSummaryAtExit(void) {
   // If case crashes, count it as fail, and accumulate finishrun
   if(UTest::retStatistics.finishrun != UTest::utestList->size()) {
@@ -113,7 +114,6 @@  UTest::UTest(Function fn, const char *name, bool isBenchMark, bool haveIssue, bo
     utestList = new vector<UTest>;
 
     catch_signal();
-    atexit(runSummaryAtExit);
   }
   utestList->push_back(*this);
 }

Comments

It seems gcc/clang/icc support __attribute__((destructor)), but still have two comments. Thanks for your contribution.

> -----Original Message-----

> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of

> Koop Mast

> Sent: Thursday, August 27, 2015 18:45

> To: beignet@lists.freedesktop.org

> Cc: Koop Mast

> Subject: [Beignet] [PATCH] Use __attribute__((destructor)), not atexit(3).

> 

> On Linux, atexit(3) registered functions are called at program exit or during

> module unload. The latter is a Glibc extension not supported by FreeBSD.

> This means that, on FreeBSD, the registered function could be called after

> the module was unloaded, causing the application to crash.

> ---

>  backend/src/backend/gen_insn_selection.cpp | 2 +-

>  backend/src/sys/alloc.cpp                  | 4 ++--

>  src/performance.c                          | 7 +------

>  utests/utest.cpp                           | 2 +-

>  4 files changed, 5 insertions(+), 10 deletions(-)

> 

> diff --git a/backend/src/backend/gen_insn_selection.cpp

> b/backend/src/backend/gen_insn_selection.cpp

> index b84bb4b..e90fd8d 100644

> --- a/backend/src/backend/gen_insn_selection.cpp

> +++ b/backend/src/backend/gen_insn_selection.cpp

> @@ -1776,11 +1776,11 @@ namespace gbe

> 

>    // Boiler plate to initialize the selection library at c++ pre-main

>    static SelectionLibrary *selLib = NULL;

> +  __attribute__((destructor))

>    static void destroySelectionLibrary(void) { GBE_DELETE(selLib); }

Only register atexit in function SelectionLibraryInitializer, but will always call destroySelectionLibrary when unload or exit here, so must add selLib check.


>    static struct SelectionLibraryInitializer {

>      SelectionLibraryInitializer(void) {

>        selLib = GBE_NEW_NO_ARG(SelectionLibrary);

> -      atexit(destroySelectionLibrary);

>      }

>    } selectionLibraryInitializer;

> 

> diff --git a/backend/src/sys/alloc.cpp b/backend/src/sys/alloc.cpp index

> 08dc7b1..30dc887 100644

> --- a/backend/src/sys/alloc.cpp

> +++ b/backend/src/sys/alloc.cpp

> @@ -140,16 +140,17 @@ namespace gbe

>    static bool isMutexInitializing = true;

>    static size_t memDebuggerCurrSize(0u);

>    static size_t memDebuggerMaxSize(0u);

> +  __attribute__((destructor))

>    static void SizeMutexDeallocate(void) { if (sizeMutex) delete sizeMutex; }

>    static void SizeMutexAllocate(void) {

>      if (sizeMutex == NULL && isMutexInitializing == false) {

>        isMutexInitializing = true;

>        sizeMutex = new MutexSys;

> -      atexit(SizeMutexDeallocate);

>      }

>    }

> 

>    /*! Stop the memory debugger */

> +  __attribute__((destructor))

>    static void MemDebuggerEnd(void) {

>      MemDebugger *_debug = memDebugger;

>      memDebugger = NULL;

Also need add memDebugger check.


> @@ -172,7 +173,6 @@ namespace gbe

>    /*! Start the memory debugger */

>    static void MemDebuggerStart(void) {

>      if (memDebugger == NULL) {

> -      atexit(MemDebuggerEnd);

>        memDebugger = new MemDebugger;

>      }

>    }

> diff --git a/src/performance.c b/src/performance.c index 85cd481..15acded

> 100644

> --- a/src/performance.c

> +++ b/src/performance.c

> @@ -37,7 +37,6 @@ typedef struct storage

> 

> 

>  static storage record;

> -static int atexit_registered = 0;

> 

> 

>  static context_storage_node * prev_context_pointer = NULL; @@ -170,6

> +169,7 @@ static int cmp(const void *a, const void *b)

>      return 0;

>  }

> 

> +__attribute__((destructor))

>  static void print_time_info()

>  {

>    context_storage_node *p_context = record.context_storage; @@ -273,11

> +273,6 @@ static void print_time_info()

> 

>  static void insert(cl_context context, const char *kernel_name, const char

> *build_opt, float time)  {

> -  if(!atexit_registered)

> -  {

> -    atexit_registered = 1;

> -    atexit(print_time_info);

> -  }

>    context_storage_node *p_context = find_context(context);

>    kernel_storage_node *p_kernel = find_kernel(p_context, kernel_name,

> build_opt);

>    prev_context_pointer = p_context;

> diff --git a/utests/utest.cpp b/utests/utest.cpp index 0a03d8b..3d6e001

> 100644

> --- a/utests/utest.cpp

> +++ b/utests/utest.cpp

> @@ -44,6 +44,7 @@ vector<UTest> *UTest::utestList = NULL;  RStatistics

> UTest::retStatistics;

> 

>  void releaseUTestList(void) { delete UTest::utestList; }

> +__attribute__((destructor))

>  void runSummaryAtExit(void) {

>    // If case crashes, count it as fail, and accumulate finishrun

>    if(UTest::retStatistics.finishrun != UTest::utestList->size()) { @@ -113,7

> +114,6 @@ UTest::UTest(Function fn, const char *name, bool isBenchMark,

> bool haveIssue, bo

>      utestList = new vector<UTest>;

> 

>      catch_signal();

> -    atexit(runSummaryAtExit);

>    }

>    utestList->push_back(*this);

>  }

> --

> 2.4.6

> 

> _______________________________________________

> Beignet mailing list

> Beignet@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/beignet