434190 - SharedStub assembly dangerously takes nsXPTCStubBase object from caller stack and can lead to crashes

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Mike Hommey [:glandium]]

This happens on hppa, but seeing the code, could happen on several other architectures, depending on how compilers work.

Depending on optimization level, the nsXPTCStubBase::Stubnnn stack frame can be of zero size. On hppa, here is what can be seen:
with -O0:
_ZN14nsXPTCStubBase7Stub249Ev:
        .PROC
        .CALLINFO FRAME=64,CALLS,SAVE_RP,SAVE_SP,ENTRY_GR=4
with -02 or -Os:
_ZN14nsXPTCStubBase7Stub249Ev:
        .PROC
        .CALLINFO FRAME=0,NO_CALLS,SAVE_RP

At the same time, the assembly code in xptcstubs_asm_pa*.s assumes the stack frame for these stub functions to be always 64 bytes, leading the assembly code to not take the proper value for the nsXPTCStubBase object, and lead to crashes.

Comment 1

[Mike Hommey [:glandium]]

This is possibly the same crash as #18875...

Comment 2

[Mike Hommey [:glandium]]

This is also the same thing as bug #15604.

Comment 3

[Mike Hommey [:glandium]]

Actually, other architectures don't seem to be impacted.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Patch

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 435823 [details] [diff] [review]
Patch

Bouncing to Benjamin, but it seems like the right thing is to make the HPPA code either handle the possible stack frame size, or add a configure test for specific behaviour we want to avoid (0-sized frame?) rather than just wiping out all optimizations here and hoping that we don't hit this problem for some other reason (like PGO).

Comment 6

[Benjamin Smedberg]

Comment on attachment 435823 [details] [diff] [review]
Patch

I'm ok with the hack, because I really don't know what form a configure test would take. Would be nice to have a more precise fix, though.

Comment 7

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/146d2f98c682