diff mbox

[revised] fix PR sanitizer/55617

Message ID 20130203202522.GA16751@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Feb. 3, 2013, 8:25 p.m. UTC
Currently darwin is unable to utilize libasan with constructors due to the lack of
constructor priority support on that target. The asan_finish_file routine inserts an
essential __asan_init into the array of constructors (via the __mod_init_func section).
However the insertion occurs at the end, and due to the lack of priority support for
constructors, these are executed from the start of the array of constructors on program
startup. This causes code any instrumented code that executes before the __asan_init
call to crash. 
   Since darwin sets...

#undef SUPPORTS_INIT_PRIORITY
#define SUPPORTS_INIT_PRIORITY 0

in gcc/config/darwin.h, all constructors are automatically set to 

#define DEFAULT_INIT_PRIORITY 65535

in gcc/collect2.c. Any code that attempts to set the constructor/destructor priority
on darwin results in a compile time error of "constructor priorities are not supported".
So asan alone should be unique in emitting priorities different from 65535 on darwin.
The attached patch uses a va_gc vector of constructor symbol/priority records to queue
this data as it is generated in calls to machopic_asm_out_constructor. Any instances of
the static constructor with priority MAX_RESERVED_INIT_PRIORITY-1 emitted by asan are 
inserted safely in the front of the vector queue which retains the original order of the
remaining constructors in the queue. The contents of the vector queue are later processed
in a new finalize_ctors routine called from darwin_file_end if necessary. The patch also
adds a g++.dg/asan/pr55617.C test case which is targeted to i?86-*-darwin* and x86_64-*-darwin*.
    The patch reduces the failures observed when running....

make -k check-g++ RUNTESTFLAGS="--target_board=unix'{-fsanitize=address}'"

from 323 to only 85 on darwin (similar to the results on linux). The cov.C testcase also
fails on gcc trunk with -fsanitize=address when recrafted into a dynamic shared library
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617#c28. This patch eliminates those
crashes. This problem doesn't extend to when the shared library or module is dlopen'd
(which works in stock gcc trunk and with this patch as well).
    The patch has been bootstrap and regression tested on x86_64-apple-darwin12.
Okay for gcc trunk?
         Jack
ps Unfortunately the flag_sort variable is unavailable inside of machopic_asm_out_constructor 
so we have to unconditionally test for priority == MAX_RESERVED_INIT_PRIORITY - 1.
/gcc

2013-02-03  Alexander Potapenko <glider@google.com>
	    Jack Howarth  <howarth@bromo.med.uc.edu>

	PR sanitizer/55617
	* config/darwin.c (machopic_asm_out_constructor): Use vector to
	queue constructors while inserting asan static constructors at front.
	(finalize_ctors): New routine to output queued constructors.
	(darwin_file_end): Use finalize_ctors.

/gcc/testsuite

2013-02-03  Alexander Potapenko <glider@google.com>
	    Jack Howarth  <howarth@bromo.med.uc.edu>

	PR sanitizer/55617
	* g++.dg/asan/pr55617.C: New test.

Comments

Richard Biener Feb. 4, 2013, 9:26 a.m. UTC | #1
On Sun, Feb 3, 2013 at 9:25 PM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
>    Currently darwin is unable to utilize libasan with constructors due to the lack of
> constructor priority support on that target. The asan_finish_file routine inserts an
> essential __asan_init into the array of constructors (via the __mod_init_func section).
> However the insertion occurs at the end, and due to the lack of priority support for
> constructors, these are executed from the start of the array of constructors on program
> startup. This causes code any instrumented code that executes before the __asan_init
> call to crash.
>    Since darwin sets...
>
> #undef SUPPORTS_INIT_PRIORITY
> #define SUPPORTS_INIT_PRIORITY 0
>
> in gcc/config/darwin.h, all constructors are automatically set to
>
> #define DEFAULT_INIT_PRIORITY 65535
>
> in gcc/collect2.c. Any code that attempts to set the constructor/destructor priority
> on darwin results in a compile time error of "constructor priorities are not supported".
> So asan alone should be unique in emitting priorities different from 65535 on darwin.
> The attached patch uses a va_gc vector of constructor symbol/priority records to queue
> this data as it is generated in calls to machopic_asm_out_constructor. Any instances of
> the static constructor with priority MAX_RESERVED_INIT_PRIORITY-1 emitted by asan are
> inserted safely in the front of the vector queue which retains the original order of the
> remaining constructors in the queue. The contents of the vector queue are later processed
> in a new finalize_ctors routine called from darwin_file_end if necessary. The patch also
> adds a g++.dg/asan/pr55617.C test case which is targeted to i?86-*-darwin* and x86_64-*-darwin*.
>     The patch reduces the failures observed when running....
>
> make -k check-g++ RUNTESTFLAGS="--target_board=unix'{-fsanitize=address}'"
>
> from 323 to only 85 on darwin (similar to the results on linux). The cov.C testcase also
> fails on gcc trunk with -fsanitize=address when recrafted into a dynamic shared library
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617#c28. This patch eliminates those
> crashes. This problem doesn't extend to when the shared library or module is dlopen'd
> (which works in stock gcc trunk and with this patch as well).
>     The patch has been bootstrap and regression tested on x86_64-apple-darwin12.
> Okay for gcc trunk?

See my other reply - this does not work for multiple TUs.

Richard.

>          Jack
> ps Unfortunately the flag_sort variable is unavailable inside of machopic_asm_out_constructor
> so we have to unconditionally test for priority == MAX_RESERVED_INIT_PRIORITY - 1.
>
diff mbox

Patch

Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 195701)
+++ gcc/config/darwin.c	(working copy)
@@ -83,6 +83,13 @@  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 {
+  rtx symbol;
+  int priority;		/* constructor priority */
+} ctor_record;
+
+static GTY(()) vec<ctor_record, va_gc> *ctors = 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_
    functions).  */
@@ -1708,15 +1715,33 @@  machopic_select_rtx_section (enum machin
 void
 machopic_asm_out_constructor (rtx symbol, int priority ATTRIBUTE_UNUSED)
 {
+  ctor_record new_elt = {symbol, priority};
+  /* Force asan static constructors to front of constructor queue. */
+  if (priority == MAX_RESERVED_INIT_PRIORITY - 1)
+    vec_safe_insert(ctors, 0, new_elt); 
+  else 
+    vec_safe_push (ctors, new_elt);
+
+  if (! MACHOPIC_INDIRECT)
+    fprintf (asm_out_file, ".reference .constructors_used\n");
+}
+
+static void 
+finalize_ctors()
+{
+  unsigned int i;
+  ctor_record *elt;
+ 
   if (MACHOPIC_INDIRECT)
     switch_to_section (darwin_sections[mod_init_section]);
   else
     switch_to_section (darwin_sections[constructor_section]);
-  assemble_align (POINTER_SIZE);
-  assemble_integer (symbol, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
 
-  if (! MACHOPIC_INDIRECT)
-    fprintf (asm_out_file, ".reference .constructors_used\n");
+  FOR_EACH_VEC_SAFE_ELT (ctors, i, elt)
+    {
+      assemble_align (POINTER_SIZE);
+      assemble_integer (elt->symbol, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
+    }
 }
 
 void
@@ -2762,6 +2787,8 @@  darwin_file_start (void)
 void
 darwin_file_end (void)
 {
+  if (!vec_safe_is_empty (ctors))
+    finalize_ctors();
   machopic_finish (asm_out_file);
   if (strcmp (lang_hooks.name, "GNU C++") == 0)
     {
--- /dev/null	2013-02-02 10:53:51.000000000 -0500
+++ gcc/testsuite/g++.dg/asan/pr55617.C	2013-02-02 10:22:17.000000000 -0500
@@ -0,0 +1,8 @@ 
+// { dg-do run { target { i?86-*-darwin* x86_64-*-darwin* } } }
+
+struct c18 { 
+  virtual void bar() { }
+};
+c18 ret;
+int main () {
+}