[Bug target/33579] INIT_PRIORITY is broken
--- Comment #14 from mmitchel at gcc dot gnu dot org 2007-11-06 00:31 --- Subject: Bug 33579 Author: mmitchel Date: Tue Nov 6 00:30:52 2007 New Revision: 129918 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=129918 Log: PR target/33579 * tree.h (DECL_INIT_PRIORITY): Do not require DECL_HAS_INIT_PRIORITY_P. (DECL_FINI_PRIORITY): Likewise. * tree.c (decl_init_priority_lookup): Remove assert. (decl_fini_priority_insert): Likewise. * cgraphunit.c (static_ctors): Make it a VEC. (static_dtors): Likewise. (record_cdtor_fn): Adjust accordingly. (build_cdtor): Generate multiple functions for each initialization priority. (compare_ctor): New function. (compare_dtor): Likewise. (cgraph_build_cdtor_fns): Sort the functions by priority before calling build_cdtor. (cgraph_build_static_cdtor): Put the priority in the function's name. Modified: trunk/gcc/ChangeLog trunk/gcc/cgraphunit.c trunk/gcc/tree.c trunk/gcc/tree.h -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #15 from mmitchel at gcc dot gnu dot org 2007-11-06 00:43 --- Fixed in 4.3.0. -- mmitchel at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #12 from mark at codesourcery dot com 2007-11-01 16:50 --- Subject: Re: INIT_PRIORITY is broken danglin at gcc dot gnu dot org wrote: --- Comment #11 from danglin at gcc dot gnu dot org 2007-11-01 03:05 --- Mark, This is major progress. All the priority tests pass and there are no regressions on hppa2.0w-hp-hpux11.11 and hppa-unknown-linux-gnu. However, I don't think the patch is quite right. For example, in the gcc.dg/initpri1.c test, two identical routines for c1 are emitted: I don't think that's actually a bug -- except maybe its a misoptimization. The compiler's just inlining the calls to c1 from the _GLOBAL_... functions due to code in record_cdtor_fn: node-local.disregard_inline_limits = 1; I'm not sure that's a great idea -- especially with -Os! -- but I also don't think it's related to the bug I introduced. Do you agree? Thanks, -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #13 from dave at hiauly1 dot hia dot nrc dot ca 2007-11-01 19:45 --- Subject: Re: INIT_PRIORITY is broken I don't think that's actually a bug -- except maybe its a misoptimization. The compiler's just inlining the calls to c1 from the _GLOBAL_... functions due to code in record_cdtor_fn: I rechecked and the compiler's just inlining the calls. Do you agree? Yes, please go ahead with your fix. Thanks, Dave -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #11 from danglin at gcc dot gnu dot org 2007-11-01 03:05 --- Mark, This is major progress. All the priority tests pass and there are no regressions on hppa2.0w-hp-hpux11.11 and hppa-unknown-linux-gnu. However, I don't think the patch is quite right. For example, in the gcc.dg/initpri1.c test, two identical routines for c1 are emitted: .EXPORT c1,ENTRY,PRIV_LEV=3 c1: .PROC .CALLINFO FRAME=64,CALLS,SAVE_RP .ENTRY stw %r2,-20(%r30) addil LR'i-$global$,%r27 ldo 64(%r30),%r30 ldw RR'i-$global$(%r1),%r19 ldo 1(%r19),%r28 comib, 0,%r19,L$0036 stw %r28,RR'i-$global$(%r1) ldw -84(%r30),%r2 bv %r0(%r2) ldo -64(%r30),%r30 L$0036: .CALL bl abort,%r2 nop nop .EXIT .PROCEND and .EXPORT _GLOBAL__I_500_0_c1,ENTRY,PRIV_LEV=3 _GLOBAL__I_500_0_c1: .PROC .CALLINFO FRAME=64,CALLS,SAVE_RP .ENTRY stw %r2,-20(%r30) addil LR'i-$global$,%r27 ldo 64(%r30),%r30 ldw RR'i-$global$(%r1),%r19 ldo 1(%r19),%r28 comib, 0,%r19,L$0040 stw %r28,RR'i-$global$(%r1) ldw -84(%r30),%r2 bv %r0(%r2) ldo -64(%r30),%r30 L$0040: .CALL bl abort,%r2 nop nop .EXIT .PROCEND In the test, only _GLOBAL__I_500_0_c1 is actually called. I believe _GLOBAL__I_500_0_c1 should call c1, or more optimally be an alias for c1. For example, this handles the case where there is static data private to c1, and c1 is called more than once. I'm not particularly concerned about using aliases although I think it is doable with GNU as. Dave -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #9 from mmitchel at gcc dot gnu dot org 2007-10-30 16:34 --- Created an attachment (id=14442) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14442action=view) patch David -- Would you please try this patch? I have lightly tested this on a hacked-up x86 configuration, and it seems to do the right thing -- but I'm not certain. If it's not right, I expect that it is at least close; please let me know what happens. -- Mark -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #10 from dave at hiauly1 dot hia dot nrc dot ca 2007-10-30 17:47 --- Subject: Re: INIT_PRIORITY is broken Would you please try this patch? I'll give it a try when I get home tonight. Thanks, Dave -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #8 from mark at codesourcery dot com 2007-10-30 02:50 --- Subject: Re: INIT_PRIORITY is broken dave at hiauly1 dot hia dot nrc dot ca wrote: I don't think this will be too hard to implement. In cgraph_build_cdtor_fns, we need to partition/sort the static_[cd]tors by priority, and then pass each batch off to build_cdtor separately. Do you want to work on this, or do you want me to do it? At the moment, I'm finding it more and more difficult to keep up with GCC issues. No problem; it's my bug. I'll work on a patch, and send it to you to for testing -- I no longer have an HP-UX machine to work on. Thanks, -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #5 from danglin at gcc dot gnu dot org 2007-10-28 15:28 --- Changed my mind about my last comment. The new constructor priority attribute breaks the previous C++ init_priority handling using collect2. With respect to initpr1.c, it can be seen that only one GLOBAL constructor, _GLOBAL__I_0_c1, and one GLOBAL destructor, _GLOBAL__D_1_c1, are created. These respectively call all the constructors and destructors. The order of the calls is not sorted based on constructor priority, so the test fails. However, sorting the calls in _GLOBAL__I_0_c1 and _GLOBAL__D_1_c1 isn't going provide a consistent ordering across translation units. The previous init_priority mechanism was more sophisticated. A global constructor visible to collect2 was output for each constructor/ destructor priority (e.g., _GLOBAL__I$01000_foo). These would call a static function, _Z41__static_initialization_and_destruction_0ii, with two arguments, construct/destruct and priority. It would arrange to call the constructor/destructor for a given priority. Collect2 sorts the GLOBAL cdtors in terms of priority. The overall running of constructors and destructors is done using HP ld's +init and +fini arguments. At the moment, I don't see any other viable mechanism. The HP linker supports a SORT keyword on subspaces (sections) but it is only an eight bit field and it looks like we need 16 bit priorities. We also don't have the equivalent of crtbegin and crtend. To fix initpri1.c, there would have to be a mechanism similar to the old init_priority mechanism in GNU C++. -- danglin at gcc dot gnu dot org changed: What|Removed |Added CC||mark at codesourcery dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #6 from mark at codesourcery dot com 2007-10-28 22:46 --- Subject: Re: INIT_PRIORITY is broken danglin at gcc dot gnu dot org wrote: With respect to initpr1.c, it can be seen that only one GLOBAL constructor, _GLOBAL__I_0_c1, and one GLOBAL destructor, _GLOBAL__D_1_c1, are created. These respectively call all the constructors and destructors. The order of the calls is not sorted based on constructor priority, so the test fails. I'm sorry to hear of this breakage. A global constructor visible to collect2 was output for each constructor/ destructor priority (e.g., _GLOBAL__I$01000_foo). These would call a static function, _Z41__static_initialization_and_destruction_0ii, with two arguments, construct/destruct and priority. It would arrange to call the constructor/destructor for a given priority. Collect2 sorts the GLOBAL cdtors in terms of priority. The overall running of constructors and destructors is done using HP ld's +init and +fini arguments. Right now, there are two primary cases for back ends: either they support constructors and destructors (targetm.have_ctors_dtors is false, and collect2's special handling is not required), or they don't (targetm.have_ctors_dtors is false, and collect2 threads the _GLOBAL_* functions together.) I believe you're correct that my changes broke the handling of prioritized constructors in the case where we use collect2. I didn't realize that there were targets that did that before. In order to fix this, I think the correct change would be to have cgraphunit.c:cgraph_build_cdtor be smarter. In particular, it should build one function for each priority, rather than building one function for everything. Then, collect2 will work as before. I don't think this will be too hard to implement. In cgraph_build_cdtor_fns, we need to partition/sort the static_[cd]tors by priority, and then pass each batch off to build_cdtor separately. Do you want to work on this, or do you want me to do it? Thanks, -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #7 from dave at hiauly1 dot hia dot nrc dot ca 2007-10-29 00:35 --- Subject: Re: INIT_PRIORITY is broken I believe you're correct that my changes broke the handling of prioritized constructors in the case where we use collect2. I didn't realize that there were targets that did that before. It used to be that constructors and destructors were run from _main and atexit, respectively. However, this didn't work for dynamically loaded objects, so I reworked the 32-bit handling to use the linker +init and +fini options. While doing this, I noticed that somebody had added the infrastructure to run prioritized constructors using collect2. So, I enabled this feature. I think Solaris still doesn't have this. In order to fix this, I think the correct change would be to have cgraphunit.c:cgraph_build_cdtor be smarter. In particular, it should build one function for each priority, rather than building one function for everything. Then, collect2 will work as before. Sounds right. I don't think this will be too hard to implement. In cgraph_build_cdtor_fns, we need to partition/sort the static_[cd]tors by priority, and then pass each batch off to build_cdtor separately. Do you want to work on this, or do you want me to do it? At the moment, I'm finding it more and more difficult to keep up with GCC issues. Think to some extent that this is caused by the growth of the GCC project. I'm doing fulltime electrical and software design for a small company here in Ottawa. At the moment, I have two ARM based boards on my desk and a video board in the works. So, I don't expect to have any significant time off until Christmas. I appreciate that you are also extremely busy. There are two related constructor issues on this target. PR 33772 is a bug in collect2. We don't strip the version info from shared library file names because HP-UX uses .sl. It is done for shared libraries using the .so extension. This causes a problem when a library is upgraded to a newer minor version using libtool's versioning scheme. The problem is libtool links shared libraries against dependent shared libraries. When this is done, the HP linker creates undefined symbols in the library for global symbols in the dependent libraries. Thus, a GLOBAL constructor with version string in a dependent library gets hardcoded into the library. As a result, it's not possible to replace dependent libraries without breaking other libraries and applications ;( In looking at this, I realized it was a bad idea to put DWARF2 EH data in the data section. We need a lot of `_GLOBAL_F*' constructors. This is the main reason we have a problems with duplicate constructor symbols (PR 30151). I think these symbols potentially will cause version update problems since they contain a random portion. It's possible PR 30151 is fixed as I don't see any failures related to this in recent builds. I think the `_GLOBAL_F*' constructors need to be hidden in a manner similar to that on linux. However, we still need collect2 to handle prioritized constructors and destructors since the sort capability of HP ld are limited. I have a little patch for the collect2 problem but I'm still pondering whether to do something different to handle the F constructors. Thanks, Dave -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
[Bug target/33579] INIT_PRIORITY is broken
--- Comment #4 from danglin at gcc dot gnu dot org 2007-10-27 23:45 --- I believe the SUPPORTS_INIT_PRIORITY portion of my 2003 patch needs to be reverted. -- danglin at gcc dot gnu dot org changed: What|Removed |Added Component|c++ |target http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33579
