Re: [PATCH][Revised] fix PR sanitizer/55617 via qsort - dtors part

[Jack Howarth <howarth at bromo dot med dot uc dot edu>]

On Mon, Feb 04, 2013 at 08:58:45PM -0500, Jack Howarth wrote:
>    Attached is the revised patch to sort destructors within code modules
> by priority on darwin. The patch now uses a common cdtor_record structure
> type and renamed sort_cdtor_records routine. Other misc. formatting issues
> are addressed as well as well as enabling the g++.dg/asan/pr55617.C test
> on all targets. Bootstrap tested on x86_64-apple-darwin12 with no
> regressions in the asan.exp tests. Okay for gcc trunk after regression 
> testing?
>        Jack

FYI, adding in the additional patch...

Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 195747)
+++ gcc/config/darwin.h	(working copy)
@@ -912,9 +912,8 @@ extern void darwin_driver_init (unsigned
   darwin_driver_init (&decoded_options_count, &decoded_options)
 #endif
 
-/* The Apple assembler and linker do not support constructor priorities.  */
-#undef SUPPORTS_INIT_PRIORITY
-#define SUPPORTS_INIT_PRIORITY 0
+/* While Apple assembler and linker do not support constructor priorities,
+intra-module priority support is available via sort_cdtor_records. */
 
 /* When building cross-compilers (and native crosses) we shall default to 
    providing an osx-version-min of this unless overridden by the User.  */

bootstraps fine on x86_64-apple-darwin12 and now can compile the testcase
g++.dg/special/initpri1.C without errors and the resulting binary
runs fine. This change would put us on parity with clang++ that also only
supports intra-module constructor/destructor priorities. Also, I checked with
'grep -R PRIORIT *; in libstdc++ and I don't see any defines suggesting that
libstdc++ uses constructor/destructor priorities yet. I'll run a make check
and see how many test cases we need to XFAIL for this change.
            Jack

> /gcc
> 
> 2013-02-04  Alexander Potapenko <glider@google.com>
> 	    Jack Howarth  <howarth@bromo.med.uc.edu>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/55617
> 	* config/darwin.c (cdtor_record): Rename ctor_record.
> 	(sort_cdtor_records): Rename sort_ctor_records.
> 	(finalize_dtors): New routine to sort destructors by
> 	priority before use in assemble_integer.
> 	(machopic_asm_out_destructor): Use finalize_dtors if needed.
> 
> /gcc/testsuite
> 
> 2013-02-04  Alexander Potapenko <glider@google.com>
> 	    Jack Howarth  <howarth@bromo.med.uc.edu>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/55617
> 	* g++.dg/asan/pr55617.C: Run on all targets.
> 
> Index: gcc/testsuite/g++.dg/asan/pr55617.C
> ===================================================================
> --- gcc/testsuite/g++.dg/asan/pr55617.C	(revision 195743)
> +++ gcc/testsuite/g++.dg/asan/pr55617.C	(working copy)
> @@ -1,4 +1,4 @@
> -// { dg-do run { target { i?86-*-darwin* x86_64-*-darwin* } } }
> +// { dg-do run }
>  
>  struct c18 { 
>    virtual void bar() { }
> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 195743)
> +++ gcc/config/darwin.c	(working copy)
> @@ -83,13 +83,14 @@ along with GCC; see the file COPYING3.  
>     kernel) the stubs might still be required, and this will be set true.  */
>  int darwin_emit_branch_islands = false;
>  
> -typedef struct GTY(()) ctor_record {
> +typedef struct GTY(()) cdtor_record {
>    rtx symbol;
> -  int priority;		/* constructor priority */
> +  int priority;		/* [con/de]structor priority */
>    int position;		/* original position */
> -} ctor_record;
> +} cdtor_record;
>  
> -static GTY(()) vec<ctor_record, va_gc> *ctors = NULL;
> +static GTY(()) vec<cdtor_record, va_gc> *ctors = NULL;
> +static GTY(()) vec<cdtor_record, va_gc> *dtors = NULL;
>  
>  /* A flag to determine whether we are running c++ or obj-c++.  This has to be
>     settable from non-c-family contexts too (i.e. we can't use the c_dialect_
> @@ -1716,7 +1717,7 @@ machopic_select_rtx_section (enum machin
>  void
>  machopic_asm_out_constructor (rtx symbol, int priority ATTRIBUTE_UNUSED)
>  {
> -  ctor_record new_elt = {symbol, priority, vec_safe_length (ctors)};
> +  cdtor_record new_elt = {symbol, priority, vec_safe_length (ctors)};
>  
>    vec_safe_push (ctors, new_elt);
>  
> @@ -1724,27 +1725,38 @@ machopic_asm_out_constructor (rtx symbol
>      fprintf (asm_out_file, ".reference .constructors_used\n");
>  }
>  
> +void
> +machopic_asm_out_destructor (rtx symbol, int priority ATTRIBUTE_UNUSED)
> +{
> +  cdtor_record new_elt = {symbol, priority, vec_safe_length (dtors)};
> +
> +  vec_safe_push (dtors, new_elt);
> +
> +  if (! MACHOPIC_INDIRECT)
> +    fprintf (asm_out_file, ".reference .destructors_used\n");
> +}
> +
>  static int
> -sort_ctor_records (const void * a, const void * b)
> +sort_cdtor_records (const void * a, const void * b)
>  {
> -  const ctor_record *ca = (const ctor_record *)a;
> -  const ctor_record *cb = (const ctor_record *)b;
> -  if (ca->priority > cb->priority)
> +  const cdtor_record *cda = (const cdtor_record *)a;
> +  const cdtor_record *cdb = (const cdtor_record *)b;
> +  if (cda->priority > cdb->priority)
>      return 1;
> -  if (ca->priority < cb->priority)
> +  if (cda->priority < cdb->priority)
>      return -1;
> -  if (ca->position > cb->position)
> +  if (cda->position > cdb->position)
>      return 1;
> -  if (ca->position < cb->position)
> +  if (cda->position < cdb->position)
>      return -1;
>    return 0;
>  }
>  
>  static void 
> -finalize_ctors()
> +finalize_ctors ()
>  {
>    unsigned int i;
> -  ctor_record *elt;
> +  cdtor_record *elt;
>   
>    if (MACHOPIC_INDIRECT)
>      switch_to_section (darwin_sections[mod_init_section]);
> @@ -1752,7 +1764,7 @@ finalize_ctors()
>      switch_to_section (darwin_sections[constructor_section]);
>  
>    if (vec_safe_length (ctors) > 1)
> -    ctors->qsort (sort_ctor_records);
> +    ctors->qsort (sort_cdtor_records);
>    FOR_EACH_VEC_SAFE_ELT (ctors, i, elt)
>      {
>        assemble_align (POINTER_SIZE);
> @@ -1760,18 +1772,24 @@ finalize_ctors()
>      }
>  }
>  
> -void
> -machopic_asm_out_destructor (rtx symbol, int priority ATTRIBUTE_UNUSED)
> +static void
> +finalize_dtors ()
>  {
> +  unsigned int i;
> +  cdtor_record *elt;
> +
>    if (MACHOPIC_INDIRECT)
>      switch_to_section (darwin_sections[mod_term_section]);
>    else
>      switch_to_section (darwin_sections[destructor_section]);
> -  assemble_align (POINTER_SIZE);
> -  assemble_integer (symbol, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
>  
> -  if (! MACHOPIC_INDIRECT)
> -    fprintf (asm_out_file, ".reference .destructors_used\n");
> +  if (vec_safe_length (dtors) > 1)
> +    dtors->qsort (sort_cdtor_records);
> +  FOR_EACH_VEC_SAFE_ELT (dtors, i, elt)
> +    {
> +      assemble_align (POINTER_SIZE);
> +      assemble_integer (elt->symbol, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
> +    }
>  }
>  
>  void
> @@ -2804,7 +2822,9 @@ void
>  darwin_file_end (void)
>  {
>    if (!vec_safe_is_empty (ctors))
> -    finalize_ctors();
> +    finalize_ctors ();
> +  if (!vec_safe_is_empty (dtors))
> +    finalize_dtors ();
>    machopic_finish (asm_out_file);
>    if (strcmp (lang_hooks.name, "GNU C++") == 0)
>      {