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

[Yang, Rong R]

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

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Koop Mast
> Sent: Thursday, August 27, 2015 18:45
> To: beignet at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet