Status

()

defect
RESOLVED WONTFIX
12 years ago
3 years ago

People

(Reporter: sayrer, Assigned: justin.lebar+bug)

Tracking

({perf})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

12 years ago
Bug 413253 turned it off because we have too many warnings dealing with it.
OS: Mac OS X → All
Hardware: PC → All
Version: unspecified → Trunk

Updated

12 years ago
Keywords: perf

Comment 1

11 years ago
So re-enabling this is a Moz2 thing or do you plan to do this for 1.9final still?

Perhaps it would be useful to give just a few examples of the warnings in question because not one of them was mentioned in bug 413253.
I wonder if we could improve performance by selectively turning this on where it's not too much work to fix the errors.
Do we have any idea what the perf win would be if we enabled strict aliasing?  (I assume it's nontrivial to try and see since GCC will generate invalid code until we fix the warnings.)
I just ran a clean build and collected the strict aliasing warnings.  It doesn't look like there are unreasonably many to consider fixing them, especially since in some cases (e.g. breakpad) we can just turn off strict aliasing.

Here are the strict aliasing warnings* I'm getting on Linux x64:

src/gfx/thebes/gfxContext.cpp:318
src/gfx/thebes/gfxPattern.cpp:114
src/modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp:341
dist/include/jscntxt.h:2202
dist/include/jscntxt.h:2202
dist/include/jscntxt.h:2202
dist/include/jscntxt.h:2202
dist/include/jscntxt.h:2202
dist/include/jscntxt.h:2202
src/dom/base/nsGlobalWindow.cpp:1825
src/dom/base/nsDOMClassInfo.cpp:5772
src/content/canvas/src/WebGLContextGL.cpp:2672
src/content/canvas/src/WebGLContextGL.cpp:2678
src/content/xslt/src/xpath/txXPCOMExtensionFunction.cpp:421
src/content/xslt/src/xpath/txXPCOMExtensionFunction.cpp:469
src/content/xslt/src/xpath/txXPCOMExtensionFunction.cpp:515
src/layout/style/nsCSSDataBlock.cpp:148
src/layout/style/nsCSSDataBlock.cpp:158
src/toolkit/components/places/src/nsMaybeWeakPtr.cpp:73
dist/include/nsCOMPtr.h:1077
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:90
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:102
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:118
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:138
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:149
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:152
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:163
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:167
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:172
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:90
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:102
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:118
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:138
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:149
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:152
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:163
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:167
src/toolkit/crashreporter/google-breakpad/src/common/linux/http_upload.cc:172
src/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc:354

* Actually, this isn't all of them.  There are also some errors in the old HTML parser which I already turned off.
With the gcc I have (4.4.3 on Ubuntu 10.04) I'm pretty sure I see a bunch in nsTArray quite frequently.
http_upload.cc isn't even linked into Firefox, only the crash reporter client. I assume most of those are from using dlsym, since you can't cast it's void* return to a function pointer without violating strict aliasing, right?
(In reply to comment #5)
> With the gcc I have (4.4.3 on Ubuntu 10.04) I'm pretty sure I see a bunch in
> nsTArray quite frequently.

Interesting.  I just did a full clobber build, and I don't see anything referring to nsTArray.  On the other hand, I do see references to mpi/mpcpucache.c, ssl3con.c, sslsnce.c, and jarfile.c.  I didn't see any of those before.
Ahh.  I do get lots of warnings in nsTArray when I set -Wstrict-aliasing=2.  It's not clear that's the right setting, but maybe it is.  See http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options
Posted patch WIP v1 (obsolete) — Splinter Review
Just pushed this to try.  Seems to work OK on my Linux 64 machine.
Assignee: nobody → justin.lebar+bug
Status: NEW → ASSIGNED
Looking at the text of the patch, I now remember that one of the first things I did was fix a strict aliasing error in nsTArray.  dbaron, you're not going crazy.  :)
I wonder what we should do about making strict-aliasing violations an error.  I think they probably should be, but otoh, it can be hard to reproduce locally the conditions which cause the errors.  For instance, I'm getting an error at

   gfx/angle/src/compiler/preprocessor/scanner.c:230 

on try that I didn't get on my own machine, even when I used gcc-4.3.

The converse may be true as well: I imagine some people's builds will fail due to strict aliasing violations even though our trees are green.
Succeeding on Mac 32/64 debug/opt builds, Linux 32/64 debug builds.  Crashing on Linux 32/64 opt builds.

Let the fun begin.
Note that strict-aliasing errors often only appear in opt build, because the inlining operations which would cause them to become issues don't happen in debug builds.
These are the valgrind warnings I'm currently getting before FF dies while running the crashtest on Linux-64.

==12304== Thread 2:
==12304== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to uninitialised byte(s)
==12304==    at 0x4E3CF2D: ??? (syscall-template.S:82)
==12304==    by 0x5F997C8: IPC::Channel::ChannelImpl::ProcessOutgoingMessages() (ipc_channel_posix.cc:622)
==12304==    by 0x5F9A37D: IPC::Channel::ChannelImpl::Send(IPC::Message*) (ipc_channel_posix.cc:678)
==12304==    by 0x5F77ED5: MessageLoop::RunTask(Task*) (message_loop.cc:339)
==12304==    by 0x5F782FF: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (message_loop.cc:347)
==12304==    by 0x5F7859C: MessageLoop::DoWork() (message_loop.cc:447)
==12304==    by 0x5F933CA: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) (message_pump_libevent.cc:309)
==12304==    by 0x5F780A5: MessageLoop::Run() (message_loop.cc:202)
==12304==    by 0x5F83A4B: base::Thread::ThreadMain() (thread.cc:156)
==12304==    by 0x5F93B09: ThreadFunc(void*) (platform_thread_posix.cc:26)
==12304==    by 0x4E349C9: start_thread (pthread_create.c:300)
==12304==    by 0x74156FC: clone (clone.S:112)
==12304==  Address 0xe9a78a4 is not stack'd, malloc'd or (recently) free'd
==12304==
==12247== Warning: set address range perms: large range [0x39187000, 0x1b6987000) (defined)
==12247== Warning: set address range perms: large range [0x39187000, 0x1b6987000) (noaccess)
==12247== Warning: set address range perms: large range [0x1b6900000, 0x334100000) (defined)
==12247== Warning: set address range perms: large range [0x1b6900000, 0x334100000) (noaccess)
==12247== Invalid read of size 8
==12247==    at 0x583ACCE: nsINode::ReplaceOrInsertBefore(int, nsINode*, nsINode*) (nsGenericElement.cpp:4075)
==12247==    by 0x583617D: nsINode::ReplaceOrInsertBefore(int, nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) (nsGenericElement.cpp:543)
==12247==    by 0x58E3F5E: nsHTMLTableRowElement::InsertCell(int, nsIDOMHTMLElement**) (nsHTMLTableRowElement.cpp:314)
==12247==    by 0x5C092B4: nsIDOMHTMLTableRowElement_InsertCell(JSContext*, unsigned int, long*) (dom_quickstubs.cpp:19207)
==12247==    by 0x69FFED5: js_Interpret (jsops.cpp:2145)
==12247==    by 0x6A09AB6: bool Invoke<int (*)(JSContext*, JSObject*, unsigned int, long*, long*)>(JSContext*, JSFunction*, JSScript*, int (*)(JSContext*, JSObject*, unsigned int, long*, long*), js::InvokeArgsGuard const&, unsigned int) (jsinterp.cpp:602)
==12247==    by 0x6A09EEB: js_Invoke (jsinterp.cpp:693)
==12247==    by 0x6A0A821: js_InternalInvoke (jsinterp.cpp:739)
==12247==    by 0x69ABEB9: JS_CallFunctionValue (jsapi.cpp:4850)
==12247==    by 0x596FF03: nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) (nsJSEnvironment.cpp:2224)
==12247==    by 0x59AB65C: nsJSEventListener::HandleEvent(nsIDOMEvent*) (nsJSEventListener.cpp:228)
==12247==    by 0x58879C4: nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsPIDOMEventTarget*, unsigned int, nsCxPusher*) (nsEventListenerManager.cpp:1094)
==12247==  Address 0x30 is not stack'd, malloc'd or (recently) free'd
This probably isn't worth messing with for 4.3, since 4.5 will surely discover new strict aliasing violations.  Marking as depends on gcc 4.5.
Depends on: gcc4.5
...especially since the strict aliasing engine has been rewritten for 4.5.

http://gcc.gnu.org/gcc-4.5/changes.html
Talos numbers are coming in for Mac:
	
  http://goo.gl/G1Yqt

Right now, looks like a marginal win on mac 64-bit, but I'll report back once all the runs finish.

Comment 19

8 years ago
(In reply to comment #17)
> Here's a pretty good reference on strict aliasing:
> 
> http://replay.web.archive.org/20090421025015/http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_aliasing.html?

Very interesting to read. :-) In the summary it says:
"Be wary of code that requires the use of -fno-strict-aliasing (turns off strict aliasing at any level) in order to work. This is a very good indication that the code relies on aliased memory access and is likely to be dominated by poor memory access patterns." ^^
Keep in mind that the article focuses on the cell processor, which has different properties than x86 or ARM processors.
Full results (comparing four runs of the try push to 4 m-c builds) didn't push the Dromaeo needle too much.  There's a 2.3% improvement on Dromaeo V8 on Mac64 that looks like it might be statistically significant, but that's about it.

We might see bigger effects on Linux, since we're using a newer version of GCC and since 4.5 has a new strict aliasing engine (whatever that means).

At this point, I think the question is: Are we willing to take this change, which adds some ugliness in place of reinterpret_casts and which will cause builds to fail when strict aliasing is violated, in exchange for a marginal performance benefit restricted to Mac, Linux, and Android?
Is there a correctness hazard if we don't fix this bug, ignoring small perf win?

/be

Comment 23

8 years ago
(In reply to comment #21)
> Full results (comparing four runs of the try push to 4 m-c builds) didn't push
> the Dromaeo needle too much.  There's a 2.3% improvement on Dromaeo V8 on Mac64
> that looks like it might be statistically significant, but that's about it.
Whats about the improvement if we use the newer Clang instead of the older Apple GCC on Mac? Does Clang handle aliasing different than GCC?
Clang has type based alias analysis too. They don't get used as much as in current GCCs and it gives preference to the "points to analysis".

In summary, if gcc 4.5 is happy, clang will be happy too.
(In reply to comment #22)
> Is there a correctness hazard if we don't fix this bug, ignoring small perf
> win?

To the contrary, enabling -fstrict-aliasing caries a correctness hazard.  The compiler tries to give warnings where its assumptions are violated, but it uses a heuristic.

Hopefully the warnings plus our tests would find all the outstanding issues, but of course we have no evidence as to whether or not that's true.

I don't know of any correctness issues associated with leaving strict aliasing off.
Posted patch WIP v2Splinter Review
Here's the latest patch.  It passes the tests on MacOS and compiles with GCC 4.5 on Linux.  I don't think it's going to run without crashing on Linux, however.  We'll see when try comes back.
Attachment #460712 - Attachment is obsolete: true
Latest patch crashes on startup on Linux64 dereferencing a null pointer.  

#0  0x00007ffff622106d in nsScriptNameSpaceManager::FillHashWithDOMInterfaces (this=0x7fffe10c3ac0) at /build/jlebar/moz/ff-1/src/dom/base/nsScriptNameSpaceManager.cpp:259
#1  0x00007ffff6222c62 in nsScriptNameSpaceManager::Init (this=0x7fffe10c3ac0) at /build/jlebar/moz/ff-1/src/dom/base/nsScriptNameSpaceManager.cpp:413
#2  0x00007ffff61d21df in nsJSRuntime::GetNameSpaceManager () at /build/jlebar/moz/ff-1/src/dom/base/nsJSEnvironment.cpp:3836
#3  0x00007ffff6210ae2 in nsDOMClassInfo::Init () at /build/jlebar/moz/ff-1/src/dom/base/nsDOMClassInfo.cpp:2346
#4  0x00007ffff6215fb8 in NS_GetDOMClassInfoInstance (aID=<value optimized out>) at /build/jlebar/moz/ff-1/src/dom/base/nsDOMClassInfo.cpp:4980
#5  0x00007ffff5f93dc3 in nsDOMParser::QueryInterface (this=<value optimized out>, aIID=<value optimized out>, aInstancePtr=0x7fffffff7840)
    at /build/jlebar/moz/ff-1/src/content/base/src/nsDOMParser.cpp:133
#6  0x00007ffff6ac37d0 in operator() (this=0x7fffffff7bb0, qi=<value optimized out>, iid=<value optimized out>) at /build/jlebar/moz/ff-1/release-gcc-4.5/xpcom/build/nsCOMPtr.cpp:47
#7  nsCOMPtr_base::assign_from_qi (this=0x7fffffff7bb0, qi=<value optimized out>, iid=<value optimized out>) at /build/jlebar/moz/ff-1/release-gcc-4.5/xpcom/build/nsCOMPtr.cpp:96
#8  0x00007ffff65c1ae1 in operator= (ccx=..., helper=..., Scope=0x7fffe1ebb480, Interface=0x7fffe10c3a60, isGlobal=0, resultWrapper=0x7fffffff7ae0) at ../../../../dist/include/nsCOMPtr.h:681
#9  GetClassInfo (ccx=..., helper=..., Scope=0x7fffe1ebb480, Interface=0x7fffe10c3a60, isGlobal=0, resultWrapper=0x7fffffff7ae0) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcprivate.h:3128
#10 XPCWrappedNative::GetNewOrUsed (ccx=..., helper=..., Scope=0x7fffe1ebb480, Interface=0x7fffe10c3a60, isGlobal=0, resultWrapper=0x7fffffff7ae0)
    at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcwrappednative.cpp:436
#11 0x00007ffff659ff17 in XPCConvert::NativeInterface2JSObject (lccx=..., d=0x7fffe2cff698, dest=0x0, aHelper=..., iid=0x7fffe6035600, Interface=0x0, allowNativeWrapper=1, isGlobal=0,
    pErr=0x7fffffff7be0) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcconvert.cpp:1283
#12 0x00007ffff658c456 in NativeInterface2JSObject (lccx=..., aScope=0x7fffe1c90528, aCOMObj=0x7fffe10cfb30, aCache=0x0, aIID=0x7fffe6035600, aAllowWrapping=<value optimized out>,
    aVal=0x7fffe2cff698, aHolder=0x0) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/nsXPConnect.cpp:1266
#13 0x00007ffff658c624 in nsXPConnect::WrapNativeToJSVal (this=<value optimized out>, aJSContext=<value optimized out>, aScope=<value optimized out>, aCOMObj=<value optimized out>,
    aCache=<value optimized out>, aIID=<value optimized out>, aAllowWrapping=1, aVal=0x7fffe2cff698, aHolder=0x0) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/nsXPConnect.cpp:1322
#14 0x00007ffff65aab99 in nsJSCID::CreateInstance (this=0x7fffe10cba10, _retval=<value optimized out>) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcjsid.cpp:776
#15 0x00007ffff6b2799a in NS_InvokeByIndex_P (that=<value optimized out>, methodIndex=<value optimized out>, paramCount=<value optimized out>, params=<value optimized out>)
    at /build/jlebar/moz/ff-1/src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:195
#16 0x00007ffff65c0cad in Invoke (ccx=<value optimized out>, mode=<value optimized out>) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcwrappednative.cpp:3141
#17 Call (ccx=<value optimized out>, mode=<value optimized out>) at /build/jlebar/moz/ff-1/src/js/src/xpconnect/src/xpcwrappednative.cpp:2407


and JS stack trace:

0 SRCH_ENG_initFromFile() ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsSearchService.js":1067]
    bytes = undefined
    binaryInStream = undefined
    doc = undefined
    domParser = undefined
    fileInStream = [xpconnect wrapped (nsISupports, nsIInputStream, nsIFileInputStream, nsISeekableStream, nsILineInputStream)]
    this = [object Object]
1 SRCH_SVC__loadEnginesFromDir(aDir = [xpconnect wrapped nsIFile], 0, [xpconnect wrapped nsIFile]) ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsSearchService.js":2748]
    icon = undefined
    addedEngine = [object Object]
    dataType = 1
    isWritable = false
    fileExtension = "xml"
    fileURL = [xpconnect wrapped (nsISupports, nsIURI, nsIURL)]
    file = [xpconnect wrapped (nsISupports, nsIFile, nsILocalFile)]
    files = [xpconnect wrapped (nsISupports, nsISimpleEnumerator, nsIDirectoryEnumerator)]
    isInProfile = false
    this = [object Object]
2 SRCH_SVC__loadEngines() ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsSearchService.js":2575]
    rebuildCache = true
    cachePaths = /build/jlebar/moz/ff-1/valgrind/dist/bin/searchplugins
    buildID = "20110428131031"
    notInToLoad = [function]
    modifiedDir = [function]
    toLoad = [xpconnect wrapped nsIFile]
    chromeFiles = 
    chromeURIs = 
    loadFromJARs = false
    locations = [xpconnect wrapped nsISimpleEnumerator]
    loadDirs = [xpconnect wrapped nsIFile]
    cacheEnabled = true
    cache = [object Object]
    this = [object Object]
3 SearchService() ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsSearchService.js":2416]
    this = [object Object]
4 anonymous(iid = {8307b8f2-08ea-45b8-96bf-b1dc7688fe3b}, outer = null) ["resource://gre/modules/XPCOMUtils.jsm":256]
    this = [object Object]
5 XPCU_serviceLambda() ["resource://gre/modules/XPCOMUtils.jsm":187]
    this = [object Object]
6 anonymous() ["resource://gre/modules/XPCOMUtils.jsm":165]
    this = [object Object]
7 AHU_loadDefaultSearchEngine() ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsBrowserContentHandler.js":839]
    engine = undefined
    submission = undefined
    defaultEngine = undefined
    this = [object Object]
8 anonymous() ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsBrowserContentHandler.js":542]
    choice = undefined
    startPage = undefined
    ss = undefined
    haveUpdateSession = false
    overridePage = ""
    prefb = [xpconnect wrapped nsIPrefBranch]
    this = [object Object]
9 dch_handle(cmdLine = [xpconnect wrapped nsICommandLine]) ["file:///build/jlebar/moz/ff-1/release-gcc-4.5/dist/bin/components/nsBrowserContentHandler.js":818]
    URLlist = undefined
    curarg = undefined
    i = 0
    count = 0
    uri = undefined
    found = undefined
    ar = null
    urilist =
    this = [object Object]
I spent a while looking at this today.  The crash goes away and FF starts successfully if I compile xptiInterfaceInfoManager.cpp without strict aliasing.  But unlike the other places where I've had to do this, this file doesn't have any obvious strict aliasing violations.

GCC is re-ordering a load and store in xptiInterfaceInfoManager::ArrayAppender and ArrayPrefixAppender; these two functions are the only ones which generate different machine code with and without strict aliasing.  I think this has to do with the EntryToInfo function, which gets inlined.  It looks like maybe GCC is getting confused by the static cast EntryToInfo does before it returns and the getter_AddRefs in the caller, although I haven't been able to find the strict aliasing violation.

In any case, I'm going to disable strict aliasing for this file and push to try to see if Linux64 works with this one change.  Linux32 appeared to work fine with the last patch I pushed.  We can then look at the numbers and decide whether to keep going with this.
Awesome work, jlebar. Can't wait to hear the Linux outcome.

/be
That change was enough to get the Linux-64 builds to pass all the unit tests.  Here are the Talos results: http://goo.gl/PxwYJ

It's hard to draw strong conclusions, given that there's a lot of variance in the data and that our statistical analysis is not particularly sophisticated.  (I ran Talos 7 times and am comparing against as many runs from trunk, but I'm still not sure that's sufficient.)  It's also easy to fall into the green jelly beans fallacy [1] when looking at the results.

There's a marked improvement in tp4 (2.5% on Linux-32), and perhaps a sunspider improvement (2% on Linux-32 sunspider nochrome and dromaeo_sunspider).  It also looks like there's a ts_places_generated_* improvement.  But again, jelly beans...

I'm pretty sure this doesn't make things slower -- the only mean which moved substantially in the wrong direction was dromaeo DOM on Linux-32, and I think (hope?) we can chalk that up to variance.  

The one result that seems unquestionably significant is that the variance in the tryserver Talos results is far lower than in the m-c results.  This indicates that *something* is different between the two sets of numbers.  It's hard to have a lot of confidence in the data until we understand what that is causing this difference.

For the purposes of this bug, I think this data indicates that we probably want to take this change.  There's some green and no red in the compare talos page, and certainly the general trend of the data reflects an increase in speed.  There's a correctness hazard associated with strict aliasing, but I've encountered just a single gotcha in my testing, so it seems unlikely that there are many more invalid optimizations lurking which don't cause test failures.

[1] http://xkcd.com/882/
Thanks Justin!

What is the impact on code size? I would expect it to get a bit smaller.
I've filed bug 653961 on the Talos variance difference between m-c and try.
(In reply to comment #31)
> What is the impact on code size? I would expect it to get a bit smaller.

Doesn't look like a significant difference in code size.  Builds are at [1] if you want to have a look.

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-9aa7999b4cfa/try-linux/
I want to spend some more time trying to understand the xptiInterfaceInfoManager crash before putting this up for review.  But that's going to need to wait a week or two until I receive a new Linux box.
The crash also goes away if I mark EntryToInfo with __attribute__((noinline)).
With EntryToInfo fixed, it now crashes in nsINode::doInsertChildAt.

I'm starting to think the correctness hazard here isn't worth the potential speedup.  But I'll bisect to see what's going on.
The first bad revision is:
changeset:   69015:3abe2f8c592e
user:        Justin Lebar <jlebar@mozilla.com>
date:        Thu Apr 28 15:49:16 2011 +0200
summary:     Bug 590181 part 2 - Switch default gcc optimize options to -O3. r=ted.

Ugh.
Realistically, this is WONTFIX.  Life is too short to figure out why -O3 breaks -fstrict-aliasing.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
FWIW, I just stumbled across this: http://patrakov.blogspot.com.au/2009/03/dont-use-old-dtoac.html

It explains how dtoa.c violates strict aliasing rules, and how to fix it.
You need to log in before you can comment on or make changes to this bug.