Closed
Bug 1170859
Opened 9 years ago
Closed 9 years ago
nslCryptoHash.updateFromStream failure called from PluginProvider.jsm on MIPS64
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: hev, Assigned: hev)
Details
Attachments
(1 file, 1 obsolete file)
999 bytes,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8614439 -
Flags: review?(nfroyd)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → r
Comment 2•9 years ago
|
||
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+
Assignee | ||
Comment 3•9 years ago
|
||
(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•9 years ago
|
||
OK, great. Please make sure the comment reflects that the sign-extension is for register values only, then. Thanks.
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8614439 -
Attachment is obsolete: true
Attachment #8614465 -
Flags: review?(nfroyd)
Updated•9 years ago
|
Attachment #8614465 -
Flags: review?(nfroyd) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/42417785fae0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•