[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #54 from mrs at gcc dot gnu.org 2013-02-11 22:36:28 UTC --- Author: mrs Date: Mon Feb 11 22:36:23 2013 New Revision: 195956 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195956 Log: 2013-02-11 Alexander Potapenko Jack Howarth Jakub Jelinek 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. testsuite: 2013-02-11 Alexander Potapenko Jack Howarth Jakub Jelinek PR sanitizer/55617 * g++.dg/asan/pr55617.C: Run on all targets. Modified: trunk/gcc/ChangeLog trunk/gcc/config/darwin.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/g++.dg/asan/pr55617.C
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 m...@gcc.gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||FIXED --- Comment #53 from mrs at gcc dot gnu.org 2013-02-04 21:10:38 UTC --- Committed revision 195737. :-) Thanks.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #52 from mrs at gcc dot gnu.org 2013-02-04 21:07:42 UTC --- Author: mrs Date: Mon Feb 4 21:07:35 2013 New Revision: 195737 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195737 Log: 2013-02-04 Alexander Potapenko Jack Howarth Jakub Jelinek PR sanitizer/55617 * g++.dg/asan/pr55617.C: New test. Added: trunk/gcc/testsuite/g++.dg/asan/pr55617.C Modified: trunk/gcc/testsuite/ChangeLog
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #51 from mrs at gcc dot gnu.org 2013-02-04 20:08:34 UTC --- Author: mrs Date: Mon Feb 4 20:08:29 2013 New Revision: 195735 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195735 Log: 2013-02-04 Alexander Potapenko Jack Howarth Jakub Jelinek PR sanitizer/55617 * config/darwin.c (sort_ctor_records): Stabilized qsort on constructor priority by using original position. (finalize_ctors): New routine to sort constructors by priority before use in assemble_integer. (machopic_asm_out_constructor): Use finalize_ctors if needed. Modified: trunk/gcc/ChangeLog trunk/gcc/config/darwin.c
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #50 from Jack Howarth 2013-02-04 17:24:40 UTC --- (In reply to comment #49) > I agree with Jakub: it's better to return back to the qsort version of the > patch, since it fixes ASan as well, but also provides better support for > priorities in general. Posted final qsort version of the patch to gcc-patches. http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00120.html
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #49 from Alexander Potapenko 2013-02-04 10:13:49 UTC --- I agree with Jakub: it's better to return back to the qsort version of the patch, since it fixes ASan as well, but also provides better support for priorities in general.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #48 from Alexander Potapenko 2013-02-04 10:11:32 UTC --- (In reply to comment #40) > if (ctor_recA->position > ctor_recB->position) > return -1; > if (ctor_recA->position < ctor_recB->position) > return 1; > > This is wrong. I think we want this reversed as well. We want original > order. > A testcase with two ctors, should in the end have the one with the lower > original position first. > > So, for me to approve it, I need to know if sorting just inside 1 translation > unit is enough to make -fsanitize=address work. I fear the answer to that is > no, and without cross translation unit support, this is just bug pushing > (obscuring a bug with a non-fix that just makes fixing it even harder). I > won't approve it if it is bug pushing. If cross has to work, then cross > shared > libraries I suspect has to work as well. In LLVM this is done without cross-TU: we just emit a call to __asan_init per TU and append it to the list of global constructors for the current module (note that the constructors' priorities on Darwin do not work across modules at all: http://llvm.org/bugs/show_bug.cgi?id=12556
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #47 from Jack Howarth 2013-02-03 15:16:50 UTC --- posted proposed patch and regression testresults at... http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00055.html http://gcc.gnu.org/ml/gcc-testresults/2013-02/msg00251.html
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #46 from Jack Howarth 2013-02-03
00:10:02 UTC ---
(In reply to comment #40)
Also with the patch in Comment 42, the failing test case converted into a
shared library loaded via dlopen works fine...
% cat libcov.C
struct c18 {
virtual void bar() { }
};
c18 ret;
% cat covmain_dl.C
#include
#include
#include
int main () {
void *lib_handle;
lib_handle = dlopen("./libcov.dylib", RTLD_LAZY);
if (!lib_handle)
{
fprintf(stderr, "%s\n", dlerror());
exit(1);
}
dlclose(lib_handle);
}
% ./dist/bin/g++-fsf-4.8 -shared -fsanitize=address covmain_dl.C
% ./dist/bin/g++-fsf-4.8 -fsanitize=address covmain_dl.C
% ./a.out
%
This also works for a libcov.C compiled into a shared module loaded in
covmain_dl.C as libcov.so...
% ./dist/bin/g++-fsf-4.8 -bundle -fsanitize=address -o libcov.so libcov.C
% ./dist/bin/g++-fsf-4.8 -fsanitize=address covmain_dl.C
% a.out
%
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #45 from Jack Howarth 2013-02-02 22:53:59 UTC --- (In reply to comment #40) Also the impact of the proposed patch in Comment 42 could be limited even further by using... if (flag_asan && priority == 99) as the test for inserting the constructors in front of the vector queue of constructors.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #44 from Jack Howarth 2013-02-02 20:41:15 UTC --- (In reply to comment #40) Doesn't the test case I showed in Comment 28 qualify as working across translaional units? That test case still compiles and runs fine with -fsanitize=address using the patch from Comment 42 but is broken in stock gcc trunk.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #43 from Jack Howarth 2013-02-02 20:19:40 UTC --- (In reply to comment #40) Actually I think we should junk sorting entirely and use the alternative approach of the patch in Comment 42. That approach should have no impact on the ordering of constructors when -fsanitize=address isn't in use. When -fsanitize=address is used, as the ctors vector is loaded up, the asan constructors will be inserted in the front of the vector.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 Jack Howarth changed: What|Removed |Added Attachment #29338|0 |1 is obsolete|| --- Comment #42 from Jack Howarth 2013-02-02 20:11:54 UTC --- Created attachment 29339 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29339 alternative approach of only inserting asan static constructor in front
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #41 from Jack Howarth 2013-02-02 20:11:07 UTC --- Created attachment 29338 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29338 alternative approach of only inserting asan static constructor in front
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 m...@gcc.gnu.org changed: What|Removed |Added CC||mrs at gcc dot gnu.org --- Comment #40 from mrs at gcc dot gnu.org 2013-02-02 19:20:31 UTC --- if (ctor_recA->position > ctor_recB->position) return -1; if (ctor_recA->position < ctor_recB->position) return 1; This is wrong. I think we want this reversed as well. We want original order. A testcase with two ctors, should in the end have the one with the lower original position first. So, for me to approve it, I need to know if sorting just inside 1 translation unit is enough to make -fsanitize=address work. I fear the answer to that is no, and without cross translation unit support, this is just bug pushing (obscuring a bug with a non-fix that just makes fixing it even harder). I won't approve it if it is bug pushing. If cross has to work, then cross shared libraries I suspect has to work as well.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #39 from Jack Howarth 2013-02-02 18:16:39 UTC --- While testing whether the single qsort was sufficient, the origin of the problem on darwin was clarified. In machopic_asm_out_constructor, after the vec_safe_push, the constructors are output as... new_elt.position = 0 new_elt.priority = 65535 new_elt.position = 1 new_elt.priority = 99 which my current patch reorders as... elt->position = 1 elt->priority = 99 elt->position = 0 priority = 65535 since darwin sets #undef SUPPORTS_INIT_PRIORITY #define SUPPORTS_INIT_PRIORITY 0 in gcc/config/darwin.h, all constructors are set to #define DEFAULT_INIT_PRIORITY 65535 in gcc/collect2.c. So all of the constructors emitted are actually of a 'higher' priority and that is why I had to reverse the sort on priority from what Jakub suggested in Comment 34. FYI, darwin doesn't compile code with priorities on constructors/destructors so they will always the default init priority... initpri2.C:5:38: error: constructor priorities are not supported
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #38 from Jakub Jelinek 2013-02-02 15:38:31 UTC --- Obviously it shouldn't be typedef in that case. Anyway, this part is not a big deal, just a nit.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #37 from Jack Howarth 2013-02-02
15:31:37 UTC ---
typedef struct GTY(()) ctor_record {
rtx symbol;
int priority; /* constructor priority */
int position; /* original position */
};
fails with...
../../gcc-4.8-20130201/gcc/config/darwin.c:90: parse error: expected '(', ')',
'GTY', or an identifier, have ';'
../../gcc-4.8-20130201/gcc/config/darwin.c:92: type `va_gc' previously defined
../../gcc-4.8-20130201/gcc/rtl.h:239: previously defined here
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #36 from Jakub Jelinek 2013-02-02
08:46:55 UTC ---
} ctor_record;
Why? }; should be enough IMHO in C++. Or does GTY still require it?
int ctor_index = -1;
... ctor_index++
What is this for? Just use vec_safe_length (ctors) instead.
int sort_ctor_records (const void * a, const void * b)
Wrong formatting:
static int
sort_ctor_records (const void *a, const void *b)
ctor_recA etc. names are just too ugly, use ca and cb instead?
void finalize_ctors() {
Again, wrong formatting:
static void
finalize_ctors ()
{
if (ctors->length() > 1)
{
ctors->qsort (sort_ctor_records);
ctors->qsort (sort_ctor_records);
}
Just use
if (vec_safe_length (ctors) > 1)
{
ctors->qsort (sort_ctor_records);
otherwise it might crash if ctors is still NULL. Also, you don't need two
qsort calls.
vec_free is probably useless that late in the compilation process, I doubt
there is any GC collection after that point, but not wrong.
if (ctors)
finalize_ctors();
This should be if (!vec_safe_is_empty (ctors))
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #35 from Jack Howarth 2013-02-02 05:51:31 UTC --- Created attachment 29334 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29334 working va_gc implementation
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #34 from Jakub Jelinek 2013-02-01 22:19:45 UTC --- Can you explain why normal qsort wouldn't do the sort in one pass? You just do if (ctor_recordA->priority > ctor_recordB->priority) return -1; if (ctor_recordA->priority < ctor_recordB->priority) return 1; if (ctor_recordA->position > ctor_recordB->position) return -1; if (ctor_recordA->postiion < ctor_recordB->position) return 1; return 0; in the comparison routine. The var names aren't very nice btw.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #33 from Jack Howarth 2013-02-01 21:45:42 UTC --- Replacing the... ctors->qsort (sort_by_ctor_priority); with... qsort(ctors, ctor_index+1, sizeof(ctor_record), sort_by_ctor_priority); appears to solve the bootstrap issues. Mike Stump suggested that we use the stable_sort from c++ to achieve the sort in one pass. Is it possible to pass a va_gc to stable_sort? In particular, how do we get first and last positions to pass as described in http://www.cplusplus.com/reference/algorithm/stable_sort/?
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #32 from Jack Howarth 2013-02-01 21:22:06 UTC --- Created attachment 29332 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29332 first attempt at va_gc implementation The attached patch is a first attempt at replacing the dynamic array with a va_gc vector. Unfortunately, this code only bootstraps fine as long the line... ctors->qsort (sort_by_ctor_priority); is commented out. With the qsort enabled, the bootstrap fails with... configure: error: cannot compute suffix of object files: cannot compile in the stage1-bubble. Reversing the comparison in sort_by_ctor_priority has no effect. The previous array based patch successfully used a call to... qsort(ctors, ctor_index+1, sizeof(ctor_record), compare_ctor_priority); Is the same qsort used in the vec implementation? Is there any examples for calling qsort for vectors with all four parameters?
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #31 from Jack Howarth 2013-02-01 16:46:35 UTC --- FYI, the proof of concept patch from Comment 27 produces no regressions in the testsuite on x86_64-apple-darwin12... http://gcc.gnu.org/ml/gcc-testresults/2013-02/msg00070.html
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #30 from Jakub Jelinek 2013-02-01 07:31:24 UTC --- Don't want to spend too much time on this, so just a few hints: 1) you want to store this in a vector (see vec.h) 2) rtxs are GC allocated, you don't want to copy_rtx it, but instead mark the structure with GTY(()), mark also the vector var with GTY(()) and make it va_gc vector (see doc/gty.texi, and grep around for GTY.*vec.*va_gc and see how they are used 3) you want a stable sort, thus sorting on priority is not enough, you need to also record the original position in the list and sort by priority first, and then by original position (so that all ctors with the same position go in the original order) 4) watch formatting, you're violating GNU Coding Conventions in several ways
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #29 from Jack Howarth 2013-02-01
05:52:13 UTC ---
The proposed patch with dynamic allocation and qsort of the ctor records by
priority field reduces the number of unexpected failures for...
sudo make -k check-g++ RUNTESTFLAGS="--target_board=unix'{-fsanitize=address}'"
from 149 down to only 85.
=== g++ Summary ===
# of expected passes51483
# of unexpected failures85
# of expected failures293
# of unresolved testcases42
# of unsupported tests888
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #28 from Jack Howarth 2013-02-01
03:00:37 UTC ---
qsort version of patch works with trivial shared lib test code of...
% cat libcov.C
struct c18 {
virtual void bar() { }
};
c18 ret;
% cat covmain.C
int main () {
}
% g++-fsf-4.8 -shared -fsanitize=address -o libcov.dylib libcov.C
% g++-fsf-4.8 -fsanitize=address covmain.C libcov.dylib
% ./a.out
%
This test case segfaults when compiled stock gcc trunk.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 Jack Howarth changed: What|Removed |Added Attachment #29324|0 |1 is obsolete|| --- Comment #27 from Jack Howarth 2013-02-01 02:34:29 UTC --- Created attachment 29325 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29325 proposed patch for dynamic allocation and qsort of ctor records by priority field.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #26 from Jack Howarth 2013-02-01 02:30:10 UTC --- Created attachment 29324 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29324 proposed patch for dynamic allocation and qsort of ctor records by priority field. proposed patch for dynamic allocation and qsort of ctor records by priority field.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #25 from Jack Howarth 2013-01-31 22:25:45 UTC --- (In reply to comment #24) > Suspect we need to use... > > ctors[ctor_index].symbol = copy_rtx(symbol); > > in machopic_asm_out_constructor although I am unclear on what need need to do > to release the memory for this copy in at the end of finalize_ctors. Perhaps we just need to add... free(ctors[i].symbol); after we assemble_integer with the ctors[i].symbol in finalize_ctors?
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #24 from Jack Howarth 2013-01-31 22:22:50 UTC --- Suspect we need to use... ctors[ctor_index].symbol = copy_rtx(symbol); in machopic_asm_out_constructor although I am unclear on what need need to do to release the memory for this copy in at the end of finalize_ctors.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #23 from Jack Howarth 2013-01-31 22:01:39 UTC --- Created attachment 29323 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29323 proposed patch for dynamic allocation Proposed patch for ctor reversal in a dynamic array. Still need to address the fact that rtx in coretypes.h is... struct rtx_def; typedef struct rtx_def *rtx; so I guess struct ctor_record should be saving the contents of the struct rtx_def instead.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #22 from Jack Howarth 2013-01-30
23:40:56 UTC ---
(In reply to comment #21)
The proposed patch reduces the number of unexpected failures in the g++
testsuite when using...
make -k check-g++ RUNTESTFLAGS="--target_board=unix'{-fsanitize=address}'"
on x86_64-apple-darwin12 from 323 to only 149 failures. Nice.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #21 from Alexander Potapenko 2013-01-30 17:30:18 UTC --- Created attachment 29309 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29309 Dummy patch that reverses the order of the constructors Attached is a hacky POC patch that intends to just reverse the order of constructors in the module. It fixes the current problem with ASan ctor being the last one (it is the first one now), yet it doesn't fix the missing support for priorities. It also contains a fixed-size array of ctors, which needs to be replaced with some dynamic structure. I also have no idea what rtx is and whether it remains allocated throughout the compilation or we're just using dangling pointers.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #20 from Alexander Potapenko 2013-01-30 17:07:25 UTC --- (In reply to comment #19) > Well, if somebody does the work and in a clean way that won't penalize targets > with sane linkers and object formats, I'm not objecting, I just am not going > to > spend time on this. If clang can handle ctor priorities right at least inside > of each individual CUs, perhaps those that care about targets which don't > support that in the linker can add similar support to gcc (what I've been > suggesting, if priorities aren't supported by linker, don't emit stuff right > away, but just queue it and at the end sort it and emit. Looks like you're right and the constructors are just being emitted by machopic_asm_out_constructor() in gcc/config/darwin.c, so ASan just has no chance to add his ctor before the default one. I suppose this can be fixed in gcc/config/darwin.c, but we don't have enough knowledge and/or cycles for this. Perhaps the right thing to do is to file a bug against the owner of that file.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #19 from Jakub Jelinek 2013-01-30 16:43:03 UTC --- Well, if somebody does the work and in a clean way that won't penalize targets with sane linkers and object formats, I'm not objecting, I just am not going to spend time on this. If clang can handle ctor priorities right at least inside of each individual CUs, perhaps those that care about targets which don't support that in the linker can add similar support to gcc (what I've been suggesting, if priorities aren't supported by linker, don't emit stuff right away, but just queue it and at the end sort it and emit.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #18 from Kostya Serebryany 2013-01-30 16:36:01 UTC --- Yea... We don't have interest in supporting gcc-asan-darwin, sorry.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #17 from Jakub Jelinek 2013-01-30 16:32:11 UTC --- Solaris doesn't support Asan in gcc, and perhaps it is time to admit that Darwin doesn't either.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #16 from Jack Howarth 2013-01-30 16:31:14 UTC --- This limitation all exists for clang on darwin... http://llvm.org/bugs/show_bug.cgi?id=12556
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #15 from Jack Howarth 2013-01-30 16:28:17 UTC --- It also seems that Solaris 2 will suffer from this issue when not using Gold... #ifndef USE_GLD /* The Solaris linker doesn't understand constructor priorities. */ #undef SUPPORTS_INIT_PRIORITY #define SUPPORTS_INIT_PRIORITY 0 #endif
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #14 from Jack Howarth 2013-01-30 15:57:11 UTC --- (In reply to comment #13) See in gcc/config/darwin.h... /* The Apple assembler and linker do not support constructor priorities. */ #undef SUPPORTS_INIT_PRIORITY #define SUPPORTS_INIT_PRIORITY 0 and http://trac.macports.org/ticket/37664 discusses this limitation as well. Can't we just latch onto the SUPPORTS_INIT_PRIORITY being defined to 0 on darwin to force __asan_init to be emiited (perhaps in the constructor)? If a single call to __asan_init would suffice shouldn't that work because the constructor should be called before the destructor in real code?
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #13 from Jakub Jelinek 2013-01-30 14:41:14 UTC --- (In reply to comment #12) > This one is a necessary one. > asan_finish_file inserts __asan_init into the array of constructors (aka > __mod_init_func section). But for some reason it is inserted at the end of > that > array, while the constructors are being executed from the start of the array > at > program startup. That's why the program crashes (because it's trying to > execute > some instrumented code that accesses the shadow memory, which isn't mapped > yet), and the real question is how do we put the new constructor first > provided > that the ctor priorities do not work well on Darwin. Guess if Darwin ignores priority, then the reason for that is that asan_finish_file which adds the ctor is called very late during compilation (and has to be, otherwise it e.g. wouldn't know if it is needed at all and what globals need to be registered). And the bug is clear, config/darwin* shouldn't ignore the priority. If the object format is lame enough and doesn't support priorities, at least the routines should ensure the right priority ordering within the same compilation unit (whether by not emitting anything right away and queueing it up for emit very late, sorted according to the priority, or something else, I don't care).
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #12 from Alexander Potapenko 2013-01-30 14:32:54 UTC --- > The question is why does... > > if (builtin_decl_implicit_p (BUILT_IN_ASAN_INIT)) > return; > > in initialize_sanitizer_builtins() not emit a __asan_init while apparently... I'm guessing initialize_sanitizer_builtins() just warms something up, but doesn't actually emit any code. IANAGCCH though. > tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT); > > in asan_finish_file() emits an apparenty unnecessary one in the wrong section. This one is a necessary one. asan_finish_file inserts __asan_init into the array of constructors (aka __mod_init_func section). But for some reason it is inserted at the end of that array, while the constructors are being executed from the start of the array at program startup. That's why the program crashes (because it's trying to execute some instrumented code that accesses the shadow memory, which isn't mapped yet), and the real question is how do we put the new constructor first provided that the ctor priorities do not work well on Darwin.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #11 from Jack Howarth 2013-01-30 14:23:51 UTC --- (In reply to comment #10) > I suppose this isn't important. __mod_term_func are destructors, and they even > aren't called in the crashing program. The question is why does... if (builtin_decl_implicit_p (BUILT_IN_ASAN_INIT)) return; in initialize_sanitizer_builtins() not emit a __asan_init while apparently... tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT); in asan_finish_file() emits an apparenty unnecessary one in the wrong section.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #10 from Alexander Potapenko 2013-01-30 12:29:00 UTC --- I suppose this isn't important. __mod_term_func are destructors, and they even aren't called in the crashing program.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #9 from Jack Howarth 2013-01-29 22:15:29 UTC --- Is it significant that in the assembly, the .mod_term_func section section (which captures the call to __asan_init) is emitted before the .mod_init_func section? LFE7: .mod_term_func .align 3 .quad __GLOBAL__sub_D_00099_0_cov.C .text __GLOBAL__sub_I_00099_1_cov.C: LFB8: pushq %rbp LCFI18: movq%rsp, %rbp LCFI19: call___asan_init movl$1, %esi leaqLASAN0(%rip), %rdi call___asan_register_globals popq%rbp LCFI20: ret LFE8: .mod_init_func .align 3 .quad __GLOBAL__sub_I_00099_1_cov.C .section __TEXT,__eh_frame,coalesced,no_toc+strip_static_syms+live_support EH_frame1: .set L$set$0,LECIE1-LSCIE1 .long L$set$0 suggest that
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #8 from Jack Howarth 2013-01-29 22:04:28 UTC --- Created attachment 29302 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29302 assembly file for reduced testcase from comment 5 generated with... g++-fsf-4.8 -fsanitize=address cov.C -o cov --save-temps on x86_64-apple-darwin12
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #7 from Alexander Potapenko 2013-01-29 11:56:02 UTC --- Here's the dump of __mod_init_func (the static ctors array): === Disassembly of section __DATA.__mod_init_func: 00011040 <__DATA.__mod_init_func>: 11040: 5c pop%rsp 11041: 0d 00 00 01 00 or $0x1,%eax 11046: 00 00 add%al,(%rax) 11048: 88 0d 00 00 01 00 mov%cl,0x1(%rip)# 10001104e <_ret+0xff6e> === -- Looks like both __GLOBAL__sub_I_00099_1_cov.cc (00010d88, which is the analog of _asan.module_ctor in Clang instrumentation) and __GLOBAL__sub_I_cov.cc (00010d5c, the original module ctor) are present in __mod_init_func, but are ordered incorrectly. I've fixed the order using bvi for OS X: === 00011040 <__DATA.__mod_init_func>: 11040: 88 0d 00 00 01 00 mov%cl,0x1(%rip)# 100011046 <_ret+0xff66> 11046: 00 00 add%al,(%rax) 11048: 5c pop%rsp 11049: 0d 00 00 01 00 or $0x1,%eax === and the resulting binary didn't segfault for me.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #6 from Alexander Potapenko 2013-01-29 09:59:09 UTC --- Looking at the disassembly I see that __asan_init is placed into some __GLOBAL__sub_I_00099_1_cov.cc function, which isn't being called at runtime (__GLOBAL__sub_I_cov.cc is called instead): 00010d31 <__Z41__static_initialization_and_destruction_0ii>: 10d31: 55 push %rbp 10d32: 48 89 e5mov%rsp,%rbp 10d35: 48 83 ec 10 sub$0x10,%rsp 10d39: 89 7d fcmov%edi,-0x4(%rbp) 10d3c: 89 75 f8mov%esi,-0x8(%rbp) 10d3f: 83 7d fc 01 cmpl $0x1,-0x4(%rbp) 10d43: 75 15 jne10d5a <__Z41__static_initialization_and_destruction_0ii+0x29> 10d45: 81 7d f8 ff ff 00 00cmpl $0x,-0x8(%rbp) 10d4c: 75 0c jne10d5a <__Z41__static_initialization_and_destruction_0ii+0x29> 10d4e: 48 8d 3d 8b 03 00 00lea0x38b(%rip),%rdi# 110e0 <_ret> 10d55: e8 9c 00 00 00 callq 10df6 <__ZN3c18C1Ev$stub> 10d5a: c9 leaveq 10d5b: c3 retq 00010d5c <__GLOBAL__sub_I_cov.cc>: 10d5c: 55 push %rbp 10d5d: 48 89 e5mov%rsp,%rbp 10d60: be ff ff 00 00 mov$0x,%esi 10d65: bf 01 00 00 00 mov$0x1,%edi 10d6a: e8 c2 ff ff ff callq 10d31 <__Z41__static_initialization_and_destruction_0ii> 10d6f: 5d pop%rbp 10d70: c3 retq 00010d71 <__GLOBAL__sub_D_00099_0_cov.cc>: 10d71: 55 push %rbp 10d72: 48 89 e5mov%rsp,%rbp 10d75: be 01 00 00 00 mov$0x1,%esi 10d7a: 48 8d 3d 1f 03 00 00lea0x31f(%rip),%rdi# 110a0 <__ZTI3c18+0x20> 10d81: e8 88 00 00 00 callq 10e0e <___asan_unregister_globals$stub> 10d86: 5d pop%rbp 10d87: c3 retq 00010d88 <__GLOBAL__sub_I_00099_1_cov.cc>: 10d88: 55 push %rbp 10d89: 48 89 e5mov%rsp,%rbp 10d8c: e8 6b 00 00 00 callq 10dfc <___asan_init$stub> 10d91: be 01 00 00 00 mov$0x1,%esi 10d96: 48 8d 3d 03 03 00 00lea0x303(%rip),%rdi# 110a0 <__ZTI3c18+0x20> 10d9d: e8 60 00 00 00 callq 10e02 <___asan_register_globals$stub> 10da2: 5d pop%rbp 10da3: c3 retq
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
Alexander Potapenko changed:
What|Removed |Added
CC||glider at google dot com
--- Comment #5 from Alexander Potapenko 2013-01-29
09:49:44 UTC ---
Here's a smaller repro for this problem:
$ cat cov.cc
struct c18 {
virtual void bar() { }
};
c18 ret;
int main () {
}
=
$ inst/bin/g++ -fsanitize=address cov.cc -o cov -g
$ gdb cov
(gdb) r
Starting program: /Users/glider/src/gcc_failures/asan_g++_failures/cov
Reading symbols for shared libraries . done
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x1000221c
0x00010dd2 in c18::c18 (this=0x110e0) at cov.cc:1
1 struct c18 {
(gdb) bt
#0 0x00010dd2 in c18::c18 (this=0x110e0) at cov.cc:1
#1 0x00010d5a in __static_initialization_and_destruction_0
(__initialize_p=1, __priority=65535) at cov.cc:4
#2 0x00010d6f in _GLOBAL__sub_I_cov.cc () at cov.cc:6
#3 0x7fff5fc13378 in
__dyld__ZN16ImageLoaderMachO18doModInitFunctionsERKN11ImageLoader11LinkContextE
()
#4 0x7fff5fc13762 in
__dyld__ZN16ImageLoaderMachO16doInitializationERKN11ImageLoader11LinkContextE
()
#5 0x7fff5fc1006e in
__dyld__ZN11ImageLoader23recursiveInitializationERKNS_11LinkContextEjRNS_21InitializerTimingListE
()
#6 0x7fff5fc0feba in
__dyld__ZN11ImageLoader15runInitializersERKNS_11LinkContextERNS_21InitializerTimingListE
()
#7 0x7fff5fc01fc0 in __dyld__ZN4dyld24initializeMainExecutableEv ()
#8 0x7fff5fc05b04 in
__dyld__ZN4dyld5_mainEPK12macho_headermiPPKcS5_S5_Pm ()
#9 0x7fff5fc01397 in
__dyld__ZN13dyldbootstrap5startEPK12macho_headeriPPKclS2_Pm ()
#10 0x7fff5fc0105e in __dyld__dyld_start ()
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617
--- Comment #4 from Jack Howarth 2012-12-08
03:14:29 UTC ---
The failing testcase in gdb appears as...
gdb ./covariant3.exe
...
(gdb) br _GLOBAL__sub_I_covariant3.C
Breakpoint 1 at 0x11ce2: file covariant3.C, line 85.
(gdb) display/i $pc
(gdb) r
Starting program: /Users/howarth/asan_g++_failures/covariant3.exe
Reading symbols for shared libraries +.
done
Breakpoint 1, _GLOBAL__sub_I_covariant3.C () at covariant3.C:85
85}
1: x/i $pc 0x11ce2 <_GLOBAL__sub_I_covariant3.C+4>:mov$0x,%esi
(gdb) s
__static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535)
at covariant3.C:85
85}
1: x/i $pc 0x11cc1 <__static_initialization_and_destruction_0+14>:cmpl
$0x1,-0x4(%rbp)
(gdb)
42c18 ret;
1: x/i $pc 0x11cd0 <__static_initialization_and_destruction_0+29>:lea
0x2029(%rip),%rdi# 0x13d00
(gdb)
c18::c18 (this=0x13d00) at covariant3.C:29
29struct c18 : c5, virtual c1 {
1: x/i $pc 0x11ef6 <_ZN3c18C1Ev+12>:mov-0x8(%rbp),%rax
(gdb)
c0::c0 (this=0x13d00) at covariant3.C:9
9struct c0 {};
1: x/i $pc 0x11e36 <_ZN2c0C2Ev+8>:pop%rbp
(gdb)
0x00011f02 in c18::c18 (this=0x13d00) at covariant3.C:29
29struct c18 : c5, virtual c1 {
1: x/i $pc 0x11f02 <_ZN3c18C1Ev+24>:lea0x1547(%rip),%rax#
0x13450 <_ZTT3c18+16>
(gdb)
c1::c1 (this=0x13d08, __vtt_parm=0x13450) at covariant3.C:10
10struct c1 : virtual c0 {
1: x/i $pc 0x11e48 <_ZN2c1C2Ev+16>:mov-0x10(%rbp),%rax
(gdb)
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x1000268a
0x00011e60 in c1::c1 (this=0x13d08, __vtt_parm=0x13450) at
covariant3.C:10
10struct c1 : virtual c0 {
1: x/i $pc 0x11e60 <_ZN2c1C2Ev+40>:movzbl (%rdx),%edx
(gdb)
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #3 from Jack Howarth 2012-12-07 15:44:16 UTC --- This might be due to the code... /* Startup code should go to startup subsection unless it is unlikely executed (this happens especially with function splitting where we can split away unnecessary parts of static constructors). */ if (startup && freq != NODE_FREQUENCY_UNLIKELY_EXECUTED) return (weak) ? darwin_sections[text_startup_coal_section] : darwin_sections[text_startup_section]; in gcc/config/darwin.c.
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #2 from Jack Howarth 2012-12-07 14:50:34 UTC --- Created attachment 28895 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28895 gdb log of stepi walk from 38th breakpoint of __dyld__ZN16ImageLoaderMachO18doModInitFunctionsERKN11ImageLoader11LinkContextE
[Bug sanitizer/55617] static constructors are not being instrumented correctly on darwin
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617 --- Comment #1 from Jack Howarth 2012-12-07 14:48:57 UTC --- Created attachment 28894 --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28894 assembly file for covariant3.C compiled with -fsanitize=address on x86_64-apple-darwin12 Assembly file produced with... g++-fsf-4.8 covariant3.C -fmessage-length=0 -std=c++98 -pedantic-errors -Wno-long-long -g -multiply_defined suppress -lm -fsanitize=address -o ./covariant3.exe --save-temps
