| Message ID | 20130203045748.GA23486@bromo.med.uc.edu |
|---|---|
| State | New |
| Headers | show |
On Sun, Feb 3, 2013 at 5:57 AM, 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 99 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? But that does not work across translation units, no? ISTR collect2 has support to handle constructor priorities all by itself (at link time, considering all inputs). I wonder why darwin cannot use that mechanism to support init priorities? Richard. > Jack > ps Unfortunately the flag_sort variable is unavailable inside of machopic_asm_out_constructor > so we have to unconditionally test for priority == 99. >
Constructor priorities on Darwin aren't supposed to work across translation units, see http://llvm.org/bugs/show_bug.cgi?id=12556: """ I was told (by Apple folks) that darwin does not support cross-unit constructor priorities, sorry. This is true for both gcc and llvm-gcc / clang. Within unit priorities are emulated on darwin (via emission order). Also, according to Nick Kledzik, constructors are executed on darwin strictly in link order. So, when you link foo.o and bar.o, then first ctors from foo.o are executed, then - from bar.o. Maybe this is even documented in some MachO docs. """ Yet it should be possible to delay the constructor emission for a single TU until all the compiler passes do their job, and then sort those constructors according to their priorities. On Mon, Feb 4, 2013 at 1:22 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Sun, Feb 3, 2013 at 5:57 AM, 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 99 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? > > But that does not work across translation units, no? ISTR collect2 has support > to handle constructor priorities all by itself (at link time, > considering all inputs). > I wonder why darwin cannot use that mechanism to support init priorities? > > Richard. > >> Jack >> ps Unfortunately the flag_sort variable is unavailable inside of machopic_asm_out_constructor >> so we have to unconditionally test for priority == 99. >>
> But that does not work across translation units, no? ISTR collect2 has support > to handle constructor priorities all by itself (at link time, > considering all inputs). > I wonder why darwin cannot use that mechanism to support init priorities? > > Richard. (resending, sorry for top-posting) Constructor priorities on Darwin aren't supposed to work across translation units, see http://llvm.org/bugs/show_bug.cgi?id=12556: """ I was told (by Apple folks) that darwin does not support cross-unit constructor priorities, sorry. This is true for both gcc and llvm-gcc / clang. Within unit priorities are emulated on darwin (via emission order). Also, according to Nick Kledzik, constructors are executed on darwin strictly in link order. So, when you link foo.o and bar.o, then first ctors from foo.o are executed, then - from bar.o. Maybe this is even documented in some MachO docs. """ Yet it should be possible to delay the constructor emission for a single TU until all the compiler passes do their job, and then sort those constructors according to their priorities.
On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: > > Okay for gcc trunk? > > But that does not work across translation units, no? ISTR collect2 has support > to handle constructor priorities all by itself (at link time, > considering all inputs). I wonder why the patch turned from initially at least supporting intra-CU support for ctor priorities into an ugly hack for asan. I guess asan doesn't care too much about inter-CU ctor priorities, it just needs its ctors to run before anything in the same CU is called (mainly the __asan_init call), other CUs either won't be asan instrumented, then it doesn't matter, or will be, but they will have their own __asan_init call. > I wonder why darwin cannot use that mechanism to support init priorities? But sure, if collect2 can be used for full init prio support, the better. Jakub
On Mon, Feb 4, 2013 at 1:38 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: >> > Okay for gcc trunk? >> >> But that does not work across translation units, no? ISTR collect2 has support >> to handle constructor priorities all by itself (at link time, >> considering all inputs). > > I wonder why the patch turned from initially at least supporting intra-CU > support for ctor priorities into an ugly hack for asan. I guess asan > doesn't care too much about inter-CU ctor priorities, it just needs its > ctors to run before anything in the same CU is called (mainly the > __asan_init call), other CUs either won't be asan instrumented, then it > doesn't matter, or will be, but they will have their own __asan_init call. Yes, I was going to ask the same question. Since other compile-time instrumentation tools (like ThreadSanitizer) will benefit from this as well, it's better to provide the intra-CU support by sorting the list of constructors.
On Mon, Feb 4, 2013 at 10:55 AM, Alexander Potapenko <glider@google.com> wrote: > On Mon, Feb 4, 2013 at 1:38 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: >>> > Okay for gcc trunk? >>> >>> But that does not work across translation units, no? ISTR collect2 has support >>> to handle constructor priorities all by itself (at link time, >>> considering all inputs). >> >> I wonder why the patch turned from initially at least supporting intra-CU >> support for ctor priorities into an ugly hack for asan. I guess asan >> doesn't care too much about inter-CU ctor priorities, it just needs its >> ctors to run before anything in the same CU is called (mainly the >> __asan_init call), other CUs either won't be asan instrumented, then it >> doesn't matter, or will be, but they will have their own __asan_init call. > > Yes, I was going to ask the same question. > Since other compile-time instrumentation tools (like ThreadSanitizer) > will benefit from this as well, it's better to provide the intra-CU > support by sorting the list of constructors. Which should be done by the middle-end then....? Not sure if "have-ctor-priority-support" is properly abstracted. Richard.
On Mon, Feb 04, 2013 at 10:38:29AM +0100, Jakub Jelinek wrote: > On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: > > > Okay for gcc trunk? > > > > But that does not work across translation units, no? ISTR collect2 has support > > to handle constructor priorities all by itself (at link time, > > considering all inputs). > > I wonder why the patch turned from initially at least supporting intra-CU > support for ctor priorities into an ugly hack for asan. I guess asan > doesn't care too much about inter-CU ctor priorities, it just needs its > ctors to run before anything in the same CU is called (mainly the > __asan_init call), other CUs either won't be asan instrumented, then it > doesn't matter, or will be, but they will have their own __asan_init call. Jakub, I switched to the simple insertion of the asan priorities for two reasons... 1) Mike seemed unconvinced that the single qsort with the proposed sort_ctor_records of... +static int +sort_ctor_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) + return 1; + if (ca->priority < cb->priority) + return -1; + if (ca->position > cb->position) + return -1; + if (ca->position < cb->position) + return 1; + return 0; +} would really be stable in absence of a second call to qsort. 2) Once I realized that darwin sets the default priority of constructors to DEFAULT_INIT_PRIORITY 65535, the desired sorting method seemed rather unclear. I assume we need to really sort these so that the priorities from MAX_INIT_PRIORITY-1 through 0 appear first in the queue and then those with MAX_INIT_PRIORITY, right? It isn't obvious how we can achieve that in sort_ctor_record with a single pass through qsort. Jack > > > I wonder why darwin cannot use that mechanism to support init priorities? > > But sure, if collect2 can be used for full init prio support, the better. > > Jakub
On Mon, Feb 04, 2013 at 09:22:27AM -0500, Jack Howarth wrote: > On Mon, Feb 04, 2013 at 10:38:29AM +0100, Jakub Jelinek wrote: > > On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: > > > > Okay for gcc trunk? > > > > > > But that does not work across translation units, no? ISTR collect2 has support > > > to handle constructor priorities all by itself (at link time, > > > considering all inputs). > > > > I wonder why the patch turned from initially at least supporting intra-CU > > support for ctor priorities into an ugly hack for asan. I guess asan > > doesn't care too much about inter-CU ctor priorities, it just needs its > > ctors to run before anything in the same CU is called (mainly the > > __asan_init call), other CUs either won't be asan instrumented, then it > > doesn't matter, or will be, but they will have their own __asan_init call. > > Jakub, > I switched to the simple insertion of the asan priorities for two reasons... > > 1) Mike seemed unconvinced that the single qsort with the proposed sort_ctor_records > of... > > +static int > +sort_ctor_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) > + return 1; > + if (ca->priority < cb->priority) > + return -1; > + if (ca->position > cb->position) > + return -1; > + if (ca->position < cb->position) > + return 1; > + return 0; > +} > > would really be stable in absence of a second call to qsort. > > 2) Once I realized that darwin sets the default priority of constructors to > DEFAULT_INIT_PRIORITY 65535, the desired sorting method seemed rather unclear. > I assume we need to really sort these so that the priorities from > MAX_INIT_PRIORITY-1 through 0 appear first in the queue and then those with > MAX_INIT_PRIORITY, right? It isn't obvious how we can achieve that in > sort_ctor_record with a single pass through qsort. > Jack Nevermind. I now see sorting priorities from lower to higher throughout is correct. I'll repost the originally proposed patch for qsort later. Jack > > > > > > I wonder why darwin cannot use that mechanism to support init priorities? > > > > But sure, if collect2 can be used for full init prio support, the better. > > > > Jakub
On Mon, Feb 04, 2013 at 09:22:27AM -0500, Jack Howarth wrote: > I switched to the simple insertion of the asan priorities for two reasons... > > 1) Mike seemed unconvinced that the single qsort with the proposed sort_ctor_records > of... > > +static int > +sort_ctor_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) > + return 1; > + if (ca->priority < cb->priority) > + return -1; > + if (ca->position > cb->position) > + return -1; Obviously this should have been return 1; > + if (ca->position < cb->position) > + return 1; and this return -1; > + return 0; > +} > > would really be stable in absence of a second call to qsort. Ugh, how can that not be stable? position is different in every vector entry, so even the return 0; case above would happen only if qsort (incorrectly) called it with two same pointers. So, the second and any further calls to qsort with the same comparison function in this case necessarily don't change anything in the array (ok, unless you have more than 4billion ctors and overflow position, or unless your OS has a buggy qsort (which wouldn't surprise me for Darwin)). > 2) Once I realized that darwin sets the default priority of constructors to > DEFAULT_INIT_PRIORITY 65535, the desired sorting method seemed rather unclear. > I assume we need to really sort these so that the priorities from > MAX_INIT_PRIORITY-1 through 0 appear first in the queue and then those with > MAX_INIT_PRIORITY, right? It isn't obvious how we can achieve that in > sort_ctor_record with a single pass through qsort. ?? You simply sort by priority ascending, and for same priorities, by position ascending. Jakub
On Mon, Feb 04, 2013 at 03:44:04PM +0100, Jakub Jelinek wrote: > On Mon, Feb 04, 2013 at 09:22:27AM -0500, Jack Howarth wrote: > > I switched to the simple insertion of the asan priorities for two reasons... > > > > 1) Mike seemed unconvinced that the single qsort with the proposed sort_ctor_records > > of... > > > > +static int > > +sort_ctor_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) > > + return 1; > > + if (ca->priority < cb->priority) > > + return -1; > > + if (ca->position > cb->position) > > + return -1; > > Obviously this should have been return 1; > > > + if (ca->position < cb->position) > > + return 1; > > and this return -1; > > > + return 0; > > +} > > > > would really be stable in absence of a second call to qsort. > > Ugh, how can that not be stable? position is different in every vector > entry, so even the return 0; case above would happen only if qsort > (incorrectly) called it with two same pointers. So, the second and any > further calls to qsort with the same comparison function in this case > necessarily don't change anything in the array (ok, unless you have more > than 4billion ctors and overflow position, or unless your OS has a buggy > qsort (which wouldn't surprise me for Darwin)). Actually don't we need... static int sort_ctor_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) return 1; if (ca->priority < cb->priority) return -1; if ((ca->priority == cb->priority) && (ca->position > cb->position)) return 1; if ((ca->priority == cb->priority) && (ca->position < cb->position)) return -1; return 0; } so that the last two checks only sort the original positions of constructors for the same priority? > > > 2) Once I realized that darwin sets the default priority of constructors to > > DEFAULT_INIT_PRIORITY 65535, the desired sorting method seemed rather unclear. > > I assume we need to really sort these so that the priorities from > > MAX_INIT_PRIORITY-1 through 0 appear first in the queue and then those with > > MAX_INIT_PRIORITY, right? It isn't obvious how we can achieve that in > > sort_ctor_record with a single pass through qsort. > > ?? You simply sort by priority ascending, and for same priorities, by > position ascending. > > Jakub
On Feb 4, 2013, at 6:22 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote: > I switched to the simple insertion of the asan priorities for two reasons... > > 1) Mike seemed unconvinced that the single qsort with the proposed sort_ctor_records > of… > would really be stable in absence of a second call to qsort. This is wrong; a mis-understanding of my point. Since the latest patch is correct with respect to this point, it doesn't matter.
On Feb 4, 2013, at 1:38 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: >>> Okay for gcc trunk? >> >> But that does not work across translation units, no? ISTR collect2 has support >> to handle constructor priorities all by itself (at link time, >> considering all inputs). > > I wonder why the patch turned from initially at least supporting intra-CU > support for ctor priorities into an ugly hack for asan. I guess asan > doesn't care too much about inter-CU ctor priorities, it just needs its > ctors to run before anything in the same CU is called (mainly the > __asan_init call), other CUs either won't be asan instrumented, then it > doesn't matter, or will be, but they will have their own __asan_init call. > >> I wonder why darwin cannot use that mechanism to support init priorities? > > But sure, if collect2 can be used for full init prio support, the better. It would be nice if someone contributed full init_priority support… I'd be happy to review that. A good patch for that would add it to clang for darwin, and have gcc use that same mechanism so that we can interoperate nicely. Absent interoperability… I think it would be annoying, as then you have to have a binary incompatibility to fix the one that is wrong.
I can't see how full init_priority support can work without proper aid from ld and/or the dynamic linker. According to the Apple people, those don't treat the cross-module priorities properly, so there's little that can be done on the compiler side. On Mon, Feb 4, 2013 at 11:39 PM, Mike Stump <mikestump@comcast.net> wrote: > On Feb 4, 2013, at 1:38 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: >>>> Okay for gcc trunk? >>> >>> But that does not work across translation units, no? ISTR collect2 has support >>> to handle constructor priorities all by itself (at link time, >>> considering all inputs). >> >> I wonder why the patch turned from initially at least supporting intra-CU >> support for ctor priorities into an ugly hack for asan. I guess asan >> doesn't care too much about inter-CU ctor priorities, it just needs its >> ctors to run before anything in the same CU is called (mainly the >> __asan_init call), other CUs either won't be asan instrumented, then it >> doesn't matter, or will be, but they will have their own __asan_init call. >> >>> I wonder why darwin cannot use that mechanism to support init priorities? >> >> But sure, if collect2 can be used for full init prio support, the better. > > It would be nice if someone contributed full init_priority support… I'd be happy to review that. A good patch for that would add it to clang for darwin, and have gcc use that same mechanism so that we can interoperate nicely. Absent interoperability… I think it would be annoying, as then you have to have a binary incompatibility to fix the one that is wrong. -- Alexander Potapenko Software Engineer Google Moscow
On Wed, Feb 06, 2013 at 06:23:50PM +0400, Alexander Potapenko wrote: > I can't see how full init_priority support can work without proper aid > from ld and/or the dynamic linker. According to the Apple people, > those don't treat the cross-module priorities properly, so there's > little that can be done on the compiler side. > > On Mon, Feb 4, 2013 at 11:39 PM, Mike Stump <mikestump@comcast.net> wrote: > > On Feb 4, 2013, at 1:38 AM, Jakub Jelinek <jakub@redhat.com> wrote: > >> On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote: > >>>> Okay for gcc trunk? > >>> > >>> But that does not work across translation units, no? ISTR collect2 has support > >>> to handle constructor priorities all by itself (at link time, > >>> considering all inputs). > >> > >> I wonder why the patch turned from initially at least supporting intra-CU > >> support for ctor priorities into an ugly hack for asan. I guess asan > >> doesn't care too much about inter-CU ctor priorities, it just needs its > >> ctors to run before anything in the same CU is called (mainly the > >> __asan_init call), other CUs either won't be asan instrumented, then it > >> doesn't matter, or will be, but they will have their own __asan_init call. > >> > >>> I wonder why darwin cannot use that mechanism to support init priorities? > >> > >> But sure, if collect2 can be used for full init prio support, the better. > > > > It would be nice if someone contributed full init_priority support… I'd be happy to review that. A good patch for that would add it to clang for darwin, and have gcc use that same mechanism so that we can interoperate nicely. Absent interoperability… I think it would be annoying, as then you have to have a binary incompatibility to fix the one that is wrong. > > Alexander, I never claimed full init priority support however FSF gcc on darwin currently has no init priority support at all. Since Mike wanted to sort the destructors as well as the constructors and this achieves usable intra-module init priority support for FSF gcc darwin, I don't see why we don't take advantage of it. Especially considering that the constructors and destructors will now always be sorted anyway. Jack ps We will have one advantage over clang's init priority support as we can use -flto to combine all of the code modules (outside of libraries) into a single one for the sorting of constructors/destructors. This allows the g++.dg/special/conpr-3.C execution test case to operate properly on darwin with -flto. Again, remember that clang currently at least supports init priority on a intra-module level. I am just trying to leverage the sorting of constructors/destructors that we added for asan to achive the same level of functionality in FSF gcc on darwin. > > -- > Alexander Potapenko > Software Engineer > Google Moscow
> Alexander, > I never claimed full init priority support however FSF gcc on darwin currently > has no init priority support at all. Since Mike wanted to sort the destructors as > well as the constructors and this achieves usable intra-module init priority support > for FSF gcc darwin, I don't see why we don't take advantage of it. Especially > considering that the constructors and destructors will now always be sorted anyway. > Jack > ps We will have one advantage over clang's init priority support as we can use -flto > to combine all of the code modules (outside of libraries) into a single one for the > sorting of constructors/destructors. This allows the g++.dg/special/conpr-3.C execution > test case to operate properly on darwin with -flto. Again, remember that clang currently > at least supports init priority on a intra-module level. I am just trying to leverage > the sorting of constructors/destructors that we added for asan to achive the same > level of functionality in FSF gcc on darwin. > Jack, I understand and fully support your desire for intra-module ctor/dtor priority. My comment was meant to reply to Mike (sorry for top-posting it, again), who, as far as I understood him, wanted to see full init_priority support on Darwin, which IIUC can't be implemented without the proper linker support. LTO may help as a bandaid, but I don't think this solution scales well enough yet. Alex
On Wed, Feb 06, 2013 at 07:14:07PM +0400, Alexander Potapenko wrote: > > Alexander, > > I never claimed full init priority support however FSF gcc on darwin currently > > has no init priority support at all. Since Mike wanted to sort the destructors as > > well as the constructors and this achieves usable intra-module init priority support > > for FSF gcc darwin, I don't see why we don't take advantage of it. Especially > > considering that the constructors and destructors will now always be sorted anyway. > > Jack > > ps We will have one advantage over clang's init priority support as we can use -flto > > to combine all of the code modules (outside of libraries) into a single one for the > > sorting of constructors/destructors. This allows the g++.dg/special/conpr-3.C execution > > test case to operate properly on darwin with -flto. Again, remember that clang currently > > at least supports init priority on a intra-module level. I am just trying to leverage > > the sorting of constructors/destructors that we added for asan to achive the same > > level of functionality in FSF gcc on darwin. > > > > Jack, > I understand and fully support your desire for intra-module ctor/dtor priority. > My comment was meant to reply to Mike (sorry for top-posting it, > again), who, as far as I understood him, wanted to see full > init_priority support on Darwin, which IIUC can't be implemented > without the proper linker support. LTO may help as a bandaid, but I > don't think this solution scales well enough yet. > > Alex Alex, I have already opened a radr://13149612, "inter module constructor/destructor priority support needed on darwin", and pinged the darwin linker developer on this issue. Jack ps Once we have init priority enabled on darwin in gcc trunk, I plan to open a radar about the absence of support collating this at -O4 on clang. That should be easier to address and might get Apple looking at the state of their init priority support again. IHMO, it would be a start if they at least support inter-module priority support in clang's internal assembler. I've been lobbying Apple for sometime to provide a stand-alone gas replacement based on clang's internal assembler so we would eventually get such fixes through that.
On Wed, Feb 6, 2013 at 7:14 AM, Alexander Potapenko <glider@google.com> wrote: > > I understand and fully support your desire for intra-module ctor/dtor priority. > My comment was meant to reply to Mike (sorry for top-posting it, > again), who, as far as I understood him, wanted to see full > init_priority support on Darwin, which IIUC can't be implemented > without the proper linker support. LTO may help as a bandaid, but I > don't think this solution scales well enough yet. It could be done using collect2. Though it would be a bit tedious to write. Or it could perhaps be done via a linker plugin--the Darwin linker supports plugins. I don't know if the Darwin linker has the required plugin support, but the gold linker does. Or we could just bug the Darwin guys to support it in the linker, it's clearly a useful feature, and they do have linker support. Ian
On Wed, Feb 06, 2013 at 07:36:06AM -0800, Ian Lance Taylor wrote: > On Wed, Feb 6, 2013 at 7:14 AM, Alexander Potapenko <glider@google.com> wrote: > > > > I understand and fully support your desire for intra-module ctor/dtor priority. > > My comment was meant to reply to Mike (sorry for top-posting it, > > again), who, as far as I understood him, wanted to see full > > init_priority support on Darwin, which IIUC can't be implemented > > without the proper linker support. LTO may help as a bandaid, but I > > don't think this solution scales well enough yet. > > It could be done using collect2. Though it would be a bit tedious to write. Ian, Don't you need assembler support for the constructor init priorities to be called in the correct order when they reside in static or shared libraries? I couldn't puzzle out how collect2 can help with that issue. Jack > > Or it could perhaps be done via a linker plugin--the Darwin linker > supports plugins. I don't know if the Darwin linker has the required > plugin support, but the gold linker does. > > Or we could just bug the Darwin guys to support it in the linker, it's > clearly a useful feature, and they do have linker support. > > Ian
On Wed, Feb 6, 2013 at 7:44 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote: > On Wed, Feb 06, 2013 at 07:36:06AM -0800, Ian Lance Taylor wrote: >> On Wed, Feb 6, 2013 at 7:14 AM, Alexander Potapenko <glider@google.com> wrote: >> > >> > I understand and fully support your desire for intra-module ctor/dtor priority. >> > My comment was meant to reply to Mike (sorry for top-posting it, >> > again), who, as far as I understood him, wanted to see full >> > init_priority support on Darwin, which IIUC can't be implemented >> > without the proper linker support. LTO may help as a bandaid, but I >> > don't think this solution scales well enough yet. >> >> It could be done using collect2. Though it would be a bit tedious to write. > > Ian, > Don't you need assembler support for the constructor init priorities to be > called in the correct order when they reside in static or shared libraries? > I couldn't puzzle out how collect2 can help with that issue. collect2 sees the whole link. It can gather all the constructors together and sort them. Ian
On Feb 6, 2013, at 6:23 AM, Alexander Potapenko <glider@google.com> wrote: > I can't see how full init_priority support can work without proper aid > from ld and/or the dynamic linker. I can, but, I don't see that that matters much. It is a mere matter of software. Rough sketch, define an encoding of the priority into the object, then define a process by which that data is respected during startup. The encoding is arbitrary and doesn't much matter. Once defined, the software to use that encoding is trivial. The only hanging point is the realization that one needs to hook into the normal startup sequence. The compiler can do this by having an object in the link that appears first (first to run ctors) that runs all the high priority ctors from its actor. Now, personally, I favor having the vendor add support first, so that we get a defined abi to adhere to, but that isn't strictly required. We can drive the abi, by defining it and submitted changes to clang to adhere to it. Once in and released, I'd not expect the abi to be changed post that, that would then become the de-facto api, and de-jure, once documented. > According to the Apple people, > those don't treat the cross-module priorities properly, so there's > little that can be done on the compiler side. This is mistaken.
On Feb 6, 2013, at 7:44 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote: > Don't you need assembler support for the constructor init priorities to be > called in the correct order when they reside in static or shared libraries? No. It can be done without assembler support.
Index: gcc/config/darwin.c =================================================================== --- gcc/config/darwin.c (revision 195686) +++ 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,32 @@ machopic_select_rtx_section (enum machin void machopic_asm_out_constructor (rtx symbol, int priority ATTRIBUTE_UNUSED) { + ctor_record new_elt = {symbol, priority}; + if (priority == 99) + 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 +2786,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 () { +}
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 99 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 == 99. /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.