Bug 1304962 - fix argument processing in AARCH64 xptcall; r=froydnj
authorLaszlo Ersek <lersek@redhat.com>
Sat, 24 Sep 2016 06:36:16 +0200
changeset 315200 9f3a85f50cff5d0f3a7c32c7ad9c703b8ed7d031
parent 315199 f3800637757590adbf18a09c18b9351a2d04fa76
child 315201 493087600f4b30666ae295f5a01a70db8b78d57e
push id32563
push userihsiao@mozilla.com
push dateMon, 26 Sep 2016 11:18:33 +0000
treeherderautoland@eb840c87b5fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1304962
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1304962 - fix argument processing in AARCH64 xptcall; r=froydnj The invoke_copy_to_stack() function passes incorrect "stack_args" and "end" arguments to the alloc_word() utility function, for parameter types T_I8..T_I64, T_U8..T_U64, T_BOOL, T_CHAR and T_WCHAR. Namely, the "end" input parameter of invoke_copy_to_stack(), which is currently incorrectly passed as "end" to alloc_word(), points to the very end of the entire exchange area between _NS_InvokeByIndex() and invoke_copy_to_stack(). However, alloc_word()'s "end" parameter should point to the end of the "ireg" (integer registers) sub-area of the exchange area. That is, "ireg_end" should be passed to alloc_word() as "end". Because invoke_copy_to_stack()'s "end" input parameter is strictly greater than "ireg_end", alloc_word() will happily trample over the "freg" (floating point registers) area, on the above-mentioned type branches, given a large enough "paramCount". Similarly, as second argument, "stack_args" should be passed to alloc_word(), pointing to the next available stack slot, for spilled-over arguments. Passing "stk", which initially points to the base of the entire exchange area (and hence the base of the "ireg" area) makes no sense. The two other alloc_word() calls in the function are correct. So centralize all calls to alloc_word() to a single location -- thereby ending up with a sole call site per alloc_XXX() function --, and compute only the last argument, "word", conditionally. This fixes an obscure SIGSEGV in AARCH64 Firefox. Triggering the bug requires a target function with seven integer-like parameters (not counting the implicit "this" -- aka "that" -- parameter), followed by at least one parameter of the above buggy types. nsIOService::NewChannel2() is such a target function, for example. DONTBUILD because NPTOB
xpcom/reflect/xptcall/md/unix/xptcinvoke_aarch64.cpp
--- a/xpcom/reflect/xptcall/md/unix/xptcinvoke_aarch64.cpp
+++ b/xpcom/reflect/xptcall/md/unix/xptcinvoke_aarch64.cpp
@@ -87,46 +87,50 @@ invoke_copy_to_stack(uint64_t* stk, uint
     double *freg_args = (double *)ireg_end;
     double *freg_end  = freg_args + 8;
     uint64_t *stack_args = (uint64_t *)freg_end;
 
     // leave room for 'that' argument in x0
     ++ireg_args;
 
     for (uint32_t i = 0; i < paramCount; i++, s++) {
+        uint64_t word;
+
         if (s->IsPtrData()) {
-            alloc_word(ireg_args, stack_args, ireg_end, (uint64_t)s->ptr);
-            continue;
+            word = (uint64_t)s->ptr;
+        } else {
+            // According to the ABI, integral types that are smaller than 8
+            // bytes are to be passed in 8-byte registers or 8-byte stack
+            // slots.
+            switch (s->type) {
+                case nsXPTType::T_FLOAT:
+                    alloc_float(freg_args, stack_args, freg_end, s->val.f);
+                    continue;
+                case nsXPTType::T_DOUBLE:
+                    alloc_double(freg_args, stack_args, freg_end, s->val.d);
+                    continue;
+                case nsXPTType::T_I8:    word = s->val.i8;  break;
+                case nsXPTType::T_I16:   word = s->val.i16; break;
+                case nsXPTType::T_I32:   word = s->val.i32; break;
+                case nsXPTType::T_I64:   word = s->val.i64; break;
+                case nsXPTType::T_U8:    word = s->val.u8;  break;
+                case nsXPTType::T_U16:   word = s->val.u16; break;
+                case nsXPTType::T_U32:   word = s->val.u32; break;
+                case nsXPTType::T_U64:   word = s->val.u64; break;
+                case nsXPTType::T_BOOL:  word = s->val.b;   break;
+                case nsXPTType::T_CHAR:  word = s->val.c;   break;
+                case nsXPTType::T_WCHAR: word = s->val.wc;  break;
+                default:
+                    // all the others are plain pointer types
+                    word = reinterpret_cast<uint64_t>(s->val.p);
+                    break;
+            }
         }
-        // According to the ABI, integral types that are smaller than 8 bytes
-        // are to be passed in 8-byte registers or 8-byte stack slots.
-        switch (s->type) {
-            case nsXPTType::T_FLOAT:
-                alloc_float(freg_args, stack_args, freg_end, s->val.f);
-                break;
-            case nsXPTType::T_DOUBLE:
-                alloc_double(freg_args, stack_args, freg_end, s->val.d);
-                break;
-            case nsXPTType::T_I8:  alloc_word(ireg_args, stk, end, s->val.i8);   break;
-            case nsXPTType::T_I16: alloc_word(ireg_args, stk, end, s->val.i16);  break;
-            case nsXPTType::T_I32: alloc_word(ireg_args, stk, end, s->val.i32);  break;
-            case nsXPTType::T_I64: alloc_word(ireg_args, stk, end, s->val.i64);  break;
-            case nsXPTType::T_U8:  alloc_word(ireg_args, stk, end, s->val.u8);   break;
-            case nsXPTType::T_U16: alloc_word(ireg_args, stk, end, s->val.u16);  break;
-            case nsXPTType::T_U32: alloc_word(ireg_args, stk, end, s->val.u32);  break;
-            case nsXPTType::T_U64: alloc_word(ireg_args, stk, end, s->val.u64);  break;
-            case nsXPTType::T_BOOL: alloc_word(ireg_args, stk, end, s->val.b);   break;
-            case nsXPTType::T_CHAR: alloc_word(ireg_args, stk, end, s->val.c);   break;
-            case nsXPTType::T_WCHAR: alloc_word(ireg_args, stk, end, s->val.wc); break;
-            default:
-                // all the others are plain pointer types
-                alloc_word(ireg_args, stack_args, ireg_end,
-                           reinterpret_cast<uint64_t>(s->val.p));
-                break;
-        }
+
+        alloc_word(ireg_args, stack_args, ireg_end, word);
     }
 }
 
 extern "C" nsresult _NS_InvokeByIndex(nsISupports* that, uint32_t methodIndex,
                                       uint32_t paramCount, nsXPTCVariant* params);
 
 EXPORT_XPCOM_API(nsresult)
 NS_InvokeByIndex(nsISupports* that, uint32_t methodIndex,