1170859 - nslCryptoHash.updateFromStream failure called from PluginProvider.jsm on MIPS64

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[hev [:hev]]

Code:
let hasher = Cc["@mozilla.org/security/hash;1"].
             createInstance(Ci.nsICryptoHash);
hasher.init(Ci.nsICryptoHash.MD5);
let stringStream = Cc["@mozilla.org/io/string-input-stream;1"].
                   createInstance(Ci.nsIStringInputStream);
                   stringStream.data = aStr ? aStr : "null";
hasher.updateFromStream(stringStream, -1);  // fail, return not available

On MIPS64, a uint32_t typed variable in register is sign-extend. e.g. uint32_t t = UINT32_MAX; => $a0 = 0xffffffffffffffff; (not 0x00000000ffffffff).

Looks above code, the second argument is -1 that register value is 0xffffffffffffffff, across xpconnect bridge, in c++ context, the second argument register value is 0x00000000ffffffff.

Comment 1

[hev [:hev]]

Attachment: 0001-XPCom-MIPS64-Fix-copy-u32-type-arg-to-argument-regis.patch

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8614439 [details] [diff] [review]
0001-XPCom-MIPS64-Fix-copy-u32-type-arg-to-argument-regis.patch

Review of attachment 8614439 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/reflect/xptcall/md/unix/xptcinvoke_mips64.cpp
@@ +77,4 @@
>              break;
>          case nsXPTType::T_U32:
>              if (i < N_ARG_REGS)
> +                regs[i] = s->val.i32;

This deserves a comment, something like:

// 32-bit values need to be sign-extended, so use the signed value.

@@ +81,2 @@
>              else
>                  *d++ = s->val.u32;

Do we need to do the same thing here as well?

Comment 3

[hev [:hev]]

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #2)
> Comment on attachment 8614439 [details] [diff] [review]
> 0001-XPCom-MIPS64-Fix-copy-u32-type-arg-to-argument-regis.patch
> 
> Review of attachment 8614439 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: xpcom/reflect/xptcall/md/unix/xptcinvoke_mips64.cpp
> @@ +77,4 @@
> >              break;
> >          case nsXPTType::T_U32:
> >              if (i < N_ARG_REGS)
> > +                regs[i] = s->val.i32;
> 
> This deserves a comment, something like:
> 
> // 32-bit values need to be sign-extended, so use the signed value.
OK
> 
> @@ +81,2 @@
> >              else
> >                  *d++ = s->val.u32;
> 
> Do we need to do the same thing here as well?
I think no, c++ code load signed 32-bit variable on stack by 'lw' instruction, and unsigned 32-bit by 'lwu'.

Comment 4

[Nathan Froyd [:froydnj]]

OK, great.  Please make sure the comment reflects that the sign-extension is for register values only, then.  Thanks.

Comment 5

[hev [:hev]]

Attachment: 0001-XPCom-MIPS64-Fix-copy-u32-type-arg-to-argument-regis.patch

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/42417785fae0

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/42417785fae0