Created attachment 200005[details]
crash log
> Exception: EXC_BAD_INSTRUCTION (0x0002)
Probably misaligned stack. From the seed notes for 8F1099:
"The 16-byte stack alignment requirement in the Intel ABI is now
aggressively enforced. Applications which crash with an illegal
instruction exception are probably misaligning the stack. This should
only affect assembly code; all code built with GCC should automatically
align the stack correctly."

We have hit this problem in my group at Apple, this sounds like some assembly
somewhere, NSPR? is not 16-byte aligned.
Someone in my group took a look at this stack and suggested that
frames 1 and 14 of Thead 0 looks like a good place to start looking.
I can give more details in email and/or hook people up with OS people,
send me mail.

bsmedberg: We use "xptcinvoke_unixish_x86.cpp" not "xptcinvoke_gcc_x86_unix.cpp".
McAfee: Almost assuredly the problem is in "xptcinvoke_unixish_x86.cpp". We are
short on people who know x86 assembly, and if you know someone who could take a
look that would be great. See 297326 for what we had to do for the first Mac OS
X for Intel builds.

Firefox wasn't aligning the stack inside nsXPTCStubBase::Stub5. I've attached diffs against mozilla TOT
which do this. With this change in place I can launch DeerPark and browse the web a bit. I eventually
hit a crash inside some plugin code that isn't due to a misaligned stack, so I'm guessing that's a
separate problem in TOT.
For what it's worth, I'm not sure why nsXPTCStubBase::Stub##n has inline asm at all. As far as I can tell,
the function could be written like this:
return PrepareAndDispatch(firstArg, n, &secondArg);
You can't quite do that today because the stubs are defined as taking no arguments in
xptcstubsdecl.inc, but that's clearly wrong. They obviously do take arguments, and if they were
declared like that this code would be a lot more maintainable.

(In reply to comment #8)
> You can't quite do that today because the stubs are defined as taking no
arguments in
> xptcstubsdecl.inc, but that's clearly wrong. They obviously do take
arguments, and if they were
> declared like that this code would be a lot more maintainable.
The problem is that they take variable numbers of arguments using normal (not
varargs) calling conventions, and the numbers and types are determined by
information contained in data structures that the callee has access to.

Comment on attachment 200322[details][diff][review]
Align the stack inside nsXPTCStubBase::Stub##n
>+#define ALIGN_STACK \
>+ "andl $0xfffffff0, %%esp\n\t" \
>+ "subl $0x4, %%esp\n\t"
You could put an
"addl $0x4, %%esp\n\t" \
in as the first line of this macro to avoid wasting 16 bytes in one of the 4
possible cases (although we probably never hit that one; I suspect there's no
room for variation and if we hit that one nobody would have filed the bug)
With or without that, sr=dbaron.

Comment on attachment 200322[details][diff][review]
Align the stack inside nsXPTCStubBase::Stub##n
Requesting 1.8 approval. Consider carefully - while we don't need this fix for
FF 1.5, we will need it on the Mozilla 1.8 branch within the next 2-3 months.
It is absolutely necessary in order to get FF running on Intel Macs.
So, we either take the risk now or in a later point release. Perhaps get some
guidance on the risk factor from dbaron.

(In reply to comment #9)
> The problem is that they take variable numbers of arguments using normal (not
> varargs) calling conventions, and the numbers and types are determined by
> information contained in data structures that the callee has access to.
The weird thing about that, though, is that the stubs don't seem to pass those
extra arguments on to the function they're calling or use those extra arguments.
Maybe that's something specific to the x86 implementation -- I haven't looked
at any of the others -- but PrepareAndDispatch only takes three arguments. If
we pass fewer than two arguments it'll be passed junk, and if we pass more than
that they'll get ignored. Because we're calling PrepareAndDispatch rather than
jumping to it, it doesn't really have a way to access extra arguments unless it's
both written in assembly and doing some very nasty stuff.
Or maybe I'm missing something very important here. :) Which wouldn't be
surprising considering that I hadn't looked at this code before last night....

Magic!
or...
The stub doesn't pass arguments to PrepareAndDispatch in accordance with the
standard calling convention, it passes a pointer to the base of the arguments in
memory. The args argument that PrepareAndDispatch receives points to the
arguments that the stub received in its own stack frame. methodIndex, the same
as the stub number, informs PrepareAndDispatch of the quantity and types of the
arguments.
Other architectures work similarly, although the calling conventions are of
course different.