public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug ipa/103080] New: LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge.
@ 2021-11-04 13:48 iains at gcc dot gnu.org
  2021-11-04 14:21 ` [Bug ipa/103080] " hubicka at kam dot mff.cuni.cz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: iains at gcc dot gnu.org @ 2021-11-04 13:48 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103080

            Bug ID: 103080
           Summary: LTO alters the ordering of static
                    constructors/destructors in pass_ipa_cdtor_merge.
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: iains at gcc dot gnu.org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

consider the following :
--------
#include <stdio.h>

/* Define USE_PRIO to to test that.*/

#ifdef USE_PRIO
#define PRIO_CTOR constructor(12345)
#define PRIO_DTOR destructor(12345)
#else
#define PRIO_CTOR constructor
#define PRIO_DTOR destructor
#endif

__attribute__((constructor))
static void
startup ()
{
  fprintf(stderr, "startup\n");
}

__attribute__((destructor))
static void
shutdown ()
{
  fprintf(stderr, "shutdown\n");
}

__attribute__((PRIO_CTOR))
static void
startup1 ()
{
  fprintf(stderr, "startup1\n");
}

__attribute__((PRIO_CTOR))
static void
startup2 ()
{
  fprintf(stderr, "startup2\n");
}

__attribute__((PRIO_DTOR))
static void
shutdown1 ()
{
  fprintf(stderr, "shutdown1\n");
}

__attribute__((destructor))
static void
shutdown2 ()
{
  fprintf(stderr, "shutdown2\n");
}

int main ()
{
  printf ("42\n");
  return 0;
}

====

This initial output is (to the best of my knowledge, although we do not
document it well enough) the correct behaviour.  Essentially, we execute the
static constructors in order of declaration and the destructors in the reverse
order.  This is the most reasonable ordering from the user's perspective and
more-or-less follows what C++ does (although the static DTORs are not tied to
their CTORs - they *can* be placed together in the source to make it readable).

$ ./gcc/xgcc -Bgcc ~/ctor-dtor.c -o ct 
$ ./ct
startup
startup1
startup2
42
shutdown2
shutdown1
shutdown

-----

$ ./gcc/xgcc -Bgcc ~/ctor-dtor.c -o ct -flto
$ ./ct
startup2
startup1
startup
42
shutdown
shutdown1
shutdown2

This is not (IMO at least) correct - at the very least the program behaviour
will be different with/without LTO.

I suspect that the logic is inverted in ipa.c:compare_ctor / compare_dtor. 
There is a comment that this is intended to make it more likely that library
CTORs will run early - but ISTM that if that is needed then an alternate
mechanism must be used.

=======
similar issues when priority is in use
NOTE that the different DTOR ordering *is* intentional as can be seen from the
source above.

$ ./gcc/xgcc -Bgcc ~/ctor-dtor.c -o ct -DUSE_PRIO
$ ./ct
startup1
startup2
startup
42
shutdown2
shutdown
shutdown1
---
$ ./gcc/xgcc -Bgcc ~/ctor-dtor.c -o ct -DUSE_PRIO -flto
$ ./ct
startup2
startup1
startup
42
shutdown
shutdown2
shutdown1
======

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug ipa/103080] LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge.
  2021-11-04 13:48 [Bug ipa/103080] New: LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge iains at gcc dot gnu.org
@ 2021-11-04 14:21 ` hubicka at kam dot mff.cuni.cz
  2021-11-04 14:55 ` rguenth at gcc dot gnu.org
  2021-11-04 15:03 ` iains at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: hubicka at kam dot mff.cuni.cz @ 2021-11-04 14:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103080

--- Comment #1 from hubicka at kam dot mff.cuni.cz ---
The cdtor merging code is predating LTO - it is also used for collect2
path on targets w/o cdtor sections.

I guess the DECL_UID compare is not very safe things to do since it
depends on the tree merging decisions.  We do not record declaration
order to LTO time, however we have order of definitions which would be
cgraph_node::get (f1)->order

Honza

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug ipa/103080] LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge.
  2021-11-04 13:48 [Bug ipa/103080] New: LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge iains at gcc dot gnu.org
  2021-11-04 14:21 ` [Bug ipa/103080] " hubicka at kam dot mff.cuni.cz
@ 2021-11-04 14:55 ` rguenth at gcc dot gnu.org
  2021-11-04 15:03 ` iains at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-11-04 14:55 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103080

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
IIRC there is also an older bug about CTOR/DTOR order across multiple TUs where
with -flto be behave differently than without where I said it might be nice to
preserve linker command line order (we have that order available somewhere at
WPA time as well).

OTOH, it might be nice to diagnose CTOR/DTOR dependences between TUs as
that is really UB and at WPA time we should have IPA REFs providing such
dependences.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug ipa/103080] LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge.
  2021-11-04 13:48 [Bug ipa/103080] New: LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge iains at gcc dot gnu.org
  2021-11-04 14:21 ` [Bug ipa/103080] " hubicka at kam dot mff.cuni.cz
  2021-11-04 14:55 ` rguenth at gcc dot gnu.org
@ 2021-11-04 15:03 ` iains at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: iains at gcc dot gnu.org @ 2021-11-04 15:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103080

--- Comment #3 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to hubicka from comment #1)
> The cdtor merging code is predating LTO - it is also used for collect2
> path on targets w/o cdtor sections.

Even so, I do not see how it can work there either*** - within a single built
__GLOBAL_I_xxxxxx we would have the CTOR order reversed from the source.  There
is nothing collect2 can do to fix that ;)

> I guess the DECL_UID compare is not very safe things to do since it
> depends on the tree merging decisions.

I agree it looks very fragile to depend on DECL_UID ordering (unless there is a
strong guarantee that the UID value indicates definition order (what about
declarations that become definitions later in the TU - but with different
ordering?).

>  We do not record declaration
> order to LTO time, however we have order of definitions which would be
> cgraph_node::get (f1)->order

Presumably, something new would need to be streamed - or would the relative
ordering of the functions be preserved ?

===

*** I was also looking a while ago at making collect2 do init_prio on Darwin -
but making it work with LTO seems quite a bit of lifting still to do....

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-04 15:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 13:48 [Bug ipa/103080] New: LTO alters the ordering of static constructors/destructors in pass_ipa_cdtor_merge iains at gcc dot gnu.org
2021-11-04 14:21 ` [Bug ipa/103080] " hubicka at kam dot mff.cuni.cz
2021-11-04 14:55 ` rguenth at gcc dot gnu.org
2021-11-04 15:03 ` iains at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).