Closed Bug 1170859 Opened 6 years ago Closed 6 years ago

nslCryptoHash.updateFromStream failure called from PluginProvider.jsm on MIPS64

Categories

(Core :: XPConnect, defect)

Other
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: hev, Assigned: hev)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attachment #8614439 - Flags: review?(nfroyd)
Assignee: nobody → r
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?
Attachment #8614439 - Flags: review?(nfroyd) → review+
(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'.
OK, great.  Please make sure the comment reflects that the sign-extension is for register values only, then.  Thanks.
Attachment #8614439 - Attachment is obsolete: true
Attachment #8614465 - Flags: review?(nfroyd)
Attachment #8614465 - Flags: review?(nfroyd) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/42417785fae0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.