545843 - xptcstubs broken on ARM EABI with 64 bits arguments

Status: RESOLVED INVALID

(Core :: XPCOM, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

This is an alter-ego of bug #545747.

What currently happens in SharedStub() on ARMEABI, is that r1, r2 and r3 are pushed onto the stack, making the whole list of method arguments directly accessible through the stack. PrepareAndDispatch can then parse the stack with xpt info to get argument values.

The problem is that depending on the original stack alignment, the r2-r3 combination that can be used in cases where a 64 bits argument is used can end up misaligned on stack, in which case DOUBLEWORD_ALIGN will not align the data pointer correctly. These cases include functions such as func(long long, ...) or func(int, long long, ...). A real world example exhibiting the problem is nsINavHistoryObserver's OnVisit method.

The attached patch fixes the issue by special casing DOUBLEWORD_ALIGN when parsing the first 2 words of the stack.

Comment 1

[Mike Hommey [:glandium]]

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

It seems this patch doesn't reliably work. Sadly linking libxul.so is very long on the arm machine I have access to, so debugging this is painful. I think I'll end up writing a dummy xptcstubs test.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Patch v2

This one should be good.

It appears that ARM EABI is somehow weird: 64-bit words in argument lists must be 64-bits aligned, but not absolutely: they must be aligned relatively to the stack pointer, which is only guaranteed to be 32-bits aligned.

So, the previous patch would actually only fix the problem for 64-bit arguments in the very first arguments of the function by using an offset relative to the stack pointer, but not the others.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 426810 [details] [diff] [review]
Patch v2

This bug may actually not exist. EABI specifies that the stack must be 64-bits aligned "at public entry points", which should guarantee xptcstubs to work. I now suspect xptcinvoke to actually be the sole reason of stack alignment at 32-bits boundaries only, in which case fixing bug 545747 the right way should make this bug vanish.

Comment 4

[Mike Hommey [:glandium]]

I confirm comment 3, the bug doesn't exist. More to come in bug 545747