809931 - Static{Ref,Auto}Ptr cause static constructors despite documentation to the contrary

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

In a --enable-optimize --disable-debug x86-64 GCC 4.4.5 (Debian) build, I'm seeing the following:

00000000013314fd <global constructors keyed to ClearOnShutdown.cpp>:
 13314fd:       48 8d 15 fc 0b 46 01    lea    0x1460bfc(%rip),%rdx        # 2792100 <__dso_handle>
 1331504:       48 8d 35 8d 88 51 01    lea    0x151888d(%rip),%rsi        # 2849d98 <mozilla::ClearOnShutdown_Internal::sShutdownObservers>
 133150b:       48 8d 3d ea ff ff ff    lea    -0x16(%rip),%rdi        # 13314fc <mozilla::StaticAutoPtr<mozilla::LinkedList<mozilla::ClearOnShutdown_Internal::ShutdownObserver> >::~StaticAutoPtr()>
 1331512:       e9 91 49 3b ff          jmpq   6e5ea8 <__cxa_atexit@plt>

0000000001331d43 <global constructors keyed to nsMemoryInfoDumper.cpp>:
 1331d43:       48 8d 15 b6 03 46 01    lea    0x14603b6(%rip),%rdx        # 2792100 <__dso_handle>
 1331d4a:       48 8d 35 4f 80 51 01    lea    0x151804f(%rip),%rsi        # 2849da0 <mozilla::(anonymous namespace)::sSignalPipeWatcher>
 1331d51:       48 8d 3d f2 f9 ff ff    lea    -0x60e(%rip),%rdi        # 133174a <mozilla::StaticRefPtr<mozilla::(anonymous namespace)::SignalPipeWatcher>::~StaticRefPtr()>
 1331d58:       e9 4b 41 3b ff          jmpq   6e5ea8 <__cxa_atexit@plt>
 1331d5d:       90                      nop

so we have a static constructor which does nothing except to register a do-nothing destructor.

I'm not sure whether this is an actual bug, or just a deficiency in an older version of GCC.

Comment 1

[Justin Lebar (not reading bugmail)]

I guess we should check what our production builds do.

Comment 2

[Nathan Froyd [:froydnj]]

In a nightly x86-64 build, I see at least:

0000000001331e40 <global constructors keyed to RasterImage.cpp>:
 1331e40:       48 8d 3d 4f c8 9e 00    lea    0x9ec84f(%rip),%rdi        # 1d1e696 <nsIIdentityCryptoService::COMTypeInfo<int>::kIID+0x
326>
 1331e47:       48 83 ec 08             sub    $0x8,%rsp
 1331e4b:       e8 08 15 43 ff          callq  763358 <PR_NewLogModule@plt>
 1331e50:       48 8d 15 49 e7 88 01    lea    0x188e749(%rip),%rdx        # 2bc05a0 <__dso_handle>
 1331e57:       48 8d 35 02 9a 93 01    lea    0x1939a02(%rip),%rsi        # 2c6b860 <mozilla::image::RasterImage::DecodeWorker::sSingle
ton>
 1331e5e:       48 8d 3d 8b b7 ff ff    lea    -0x4875(%rip),%rdi        # 132d5f0 <mozilla::StaticRefPtr<mozilla::image::RasterImage::DecodeWorker>::~StaticRefPtr()>
 1331e65:       48 89 05 24 9a 93 01    mov    %rax,0x1939a24(%rip)        # 2c6b890 <gCompressedImageAccountingLog>
 1331e6c:       e8 a7 4f 42 ff          callq  756e18 <__cxa_atexit@plt>
 1331e71:       48 8d 15 28 e7 88 01    lea    0x188e728(%rip),%rdx        # 2bc05a0 <__dso_handle>
 1331e78:       48 8d 35 f1 99 93 01    lea    0x19399f1(%rip),%rsi        # 2c6b870 <mozilla::image::RasterImage::ScaleWorker::sSingleton>
 1331e7f:       48 8d 3d 7a b7 ff ff    lea    -0x4886(%rip),%rdi        # 132d600 <nsRefPtr<mozilla::image::RasterImage::ScaleWorker>::~nsRefPtr()>
 1331e86:       48 c7 05 df 99 93 01    movq   $0x0,0x19399df(%rip)        # 2c6b870 <mozilla::image::RasterImage::ScaleWorker::sSingleton>
 1331e8d:       00 00 00 00 
 1331e91:       e8 82 4f 42 ff          callq  756e18 <__cxa_atexit@plt>
 1331e96:       48 8d 15 03 e7 88 01    lea    0x188e703(%rip),%rdx        # 2bc05a0 <__dso_handle>
 1331e9d:       48 8d 35 dc 99 93 01    lea    0x19399dc(%rip),%rsi        # 2c6b880 <mozilla::image::RasterImage::DrawWorker::sSingleton>
 1331ea4:       48 8d 3d 75 b7 ff ff    lea    -0x488b(%rip),%rdi        # 132d620 <nsRefPtr<mozilla::image::RasterImage::DrawWorker>::~nsRefPtr()>
 1331eab:       48 c7 05 ca 99 93 01    movq   $0x0,0x19399ca(%rip)        # 2c6b880 <mozilla::image::RasterImage::DrawWorker::sSingleton>
 1331eb2:       00 00 00 00 
 1331eb6:       e8 5d 4f 42 ff          callq  756e18 <__cxa_atexit@plt>
 1331ebb:       48 8d 15 de e6 88 01    lea    0x188e6de(%rip),%rdx        # 2bc05a0 <__dso_handle>
 1331ec2:       48 8d 35 d7 99 93 01    lea    0x19399d7(%rip),%rsi        # 2c6b8a0 <mozilla::image::sScaleWorkerThread>
 1331ec9:       48 8d 3d 9e da f5 ff    lea    -0xa2562(%rip),%rdi        # 128f96e <nsCOMPtr<nsIThread>::~nsCOMPtr()>
 1331ed0:       48 c7 05 c5 99 93 01    movq   $0x0,0x19399c5(%rip)        # 2c6b8a0 <mozilla::image::sScaleWorkerThread>
 1331ed7:       00 00 00 00 
 1331edb:       48 83 c4 08             add    $0x8,%rsp
 1331edf:       e9 34 4f 42 ff          jmpq   756e18 <__cxa_atexit@plt>
 1331ee4:       90                      nop

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: make Static{Auto,Ref}Ptr have trivial constructors and destructors

Trivial constructors and destructors need to be defined automatically by the
compiler; these classes were declaring them with empty bodies, which made the
constructor and destructor non-trivial (!).  Trivially make them trivial.

I tried sticking MOZ_DELETE on the copy constructor declaration, but that
didn't have the desired effect (declare it unusable, but still permit the
compiler to define the default constructor automagically).  Declaring it only
in DEBUG mode seems like the next best thing.

Comment 4

[Justin Lebar (not reading bugmail)]

Comment on attachment 680296 [details] [diff] [review]
make Static{Auto,Ref}Ptr have trivial constructors and destructors

<3.  Thanks for digging into this, Nathan!

Comment 5

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b63186ca9a5

(In reply to Nathan Froyd (:froydnj) from comment #3)
> I tried sticking MOZ_DELETE on the copy constructor declaration, but that
> didn't have the desired effect (declare it unusable, but still permit the
> compiler to define the default constructor automagically).  Declaring it only
> in DEBUG mode seems like the next best thing.

This really ought to work; at least drafts of the C++ atomics proposal specify that structures shall have trivial constructors and destructors alongside deleted copy constructors and operator=.  Not sure what I was doing wrong with clang and/or g++--or if they just don't support that yet...

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4b63186ca9a5