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

RESOLVED FIXED in mozilla19

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: froydnj, Unassigned)

Tracking

unspecified
mozilla19
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
I guess we should check what our production builds do.
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
Created attachment 680296 [details] [diff] [review]
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.
Attachment #680296 - Flags: review?(justin.lebar+bug)
Comment on attachment 680296 [details] [diff] [review]
make Static{Auto,Ref}Ptr have trivial constructors and destructors

<3.  Thanks for digging into this, Nathan!
Attachment #680296 - Flags: review?(justin.lebar+bug) → review+
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...
https://hg.mozilla.org/mozilla-central/rev/4b63186ca9a5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.