ReadParam(msg, iter, uint32) is extracted to ReadParam(msg, iter, size_t/intptr_t) on Windows x86

RESOLVED FIXED in mozilla5

Status

()

RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla5
x86
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
ReadParam(msg, iter, uint32) is extraced to ReadParam(msg, iter, size_t/intptr_t) on Windows x86.  if uint32, we should not use size_t due to compatiblity of 32bit <-> 64bit IPC and reduction of IPC message size.


- Disassembled code on the latest nightly

0:044> x xul!*nsACString*::Read
66065a37 xul!IPC::ParamTraits<nsACString_internal>::Read (class IPC::Message *, void **, class nsACString_internal *)

0:044> u 66065a37
xul!IPC::ParamTraits<nsACString_internal>::Read:
66065a37 55              push    ebp
66065a38 8bec            mov     ebp,esp
66065a3a 51              push    ecx
66065a3b 56              push    esi
66065a3c 8b7508          mov     esi,dword ptr [ebp+8]
66065a3f 8d450b          lea     eax,[ebp+0Bh]
66065a42 50              push    eax
66065a43 ff750c          push    dword ptr [ebp+0Ch]
66065a46 83c604          add     esi,4
66065a49 56              push    esi
66065a4a e83026ebff      call    xul!Pickle::ReadBool (65f1807f)
66065a4f 84c0            test    al,al
66065a51 7449            je      xul!IPC::ParamTraits<nsACString_internal>::Read+0x65 (66065a9c)
66065a53 807d0b00        cmp     byte ptr [ebp+0Bh],0
66065a57 740e            je      xul!IPC::ParamTraits<nsACString_internal>::Read+0x30 (66065a67)
66065a59 6a01            push    1
66065a5b ff7510          push    dword ptr [ebp+10h]
66065a5e e86a26f3ff      call    xul!nsACString_internal::SetIsVoid (65f980cd)
66065a63 b001            mov     al,1
66065a65 eb37            jmp     xul!IPC::ParamTraits<nsACString_internal>::Read+0x67 (66065a9e)
66065a67 8d4508          lea     eax,[ebp+8]
66065a6a 50              push    eax
66065a6b ff750c          push    dword ptr [ebp+0Ch]
66065a6e 56              push    esi
66065a6f e82b25ebff      call    xul!Pickle::ReadIntPtr (65f17f9f)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <-- HERE
66065a74 84c0            test    al,al
66065a76 7424            je      xul!IPC::ParamTraits<nsACString_internal>::Read+0x65 (66065a9c)
66065a78 ff7508          push    dword ptr [ebp+8]
66065a7b 8d45fc          lea     eax,[ebp-4]
66065a7e 50              push    eax
66065a7f ff750c          push    dword ptr [ebp+0Ch]
66065a82 56              push    esi
66065a83 e88d24ebff      call    xul!Pickle::ReadBytes (65f17f15)
66065a88 84c0            test    al,al
66065a8a 7410            je      xul!IPC::ParamTraits<nsACString_internal>::Read+0x65 (66065a9c)
66065a8c ff7508          push    dword ptr [ebp+8]
66065a8f 8b55fc          mov     edx,dword ptr [ebp-4]
66065a92 8b4d10          mov     ecx,dword ptr [ebp+10h]
66065a95 e83666aaff      call    xul!nsACString_internal::Assign (65b0c0d0)
66065a9a ebc7            jmp     xul!IPC::ParamTraits<nsACString_internal>::Read+0x2c (66065a63)
66065a9c 32c0            xor     al,al
66065a9e 5e              pop     esi
66065a9f c9              leave
66065aa0 c3              ret
(Assignee)

Updated

8 years ago
Summary: ReadParam(msg, iter, uint32) is extraced to ReadParam(msg, iter, size_t/intptr_t) on Windows x86 → ReadParam(msg, iter, uint32) is extracted to ReadParam(msg, iter, size_t/intptr_t) on Windows x86

Comment 1

8 years ago
I am having difficulty imagining a hybrid 32/64 bit IPC setup.  When we build, don't we build the browser and plugin-container in the same manner?
(Assignee)

Comment 2

8 years ago
(In reply to comment #1)
> I am having difficulty imagining a hybrid 32/64 bit IPC setup.  When we build,
> don't we build the browser and plugin-container in the same manner?

64bit Windows build env builds 64-bit binary only.  But IPC protocol should be considered that 64bit process talks with 32bit process.  Example, on Mac OS X 64 bit firefox can talk with 32-bit plugin-container.

I am analyzing and testing 32bit process and 64bit process built from same source code.
(Assignee)

Comment 3

8 years ago
Created attachment 499247 [details] [diff] [review]
fix v1
Assignee: nobody → m_kato
(Assignee)

Comment 4

8 years ago
Created attachment 499612 [details] [diff] [review]
fix v1.1
Attachment #499247 - Attachment is obsolete: true
Attachment #499612 - Flags: review?(jones.chris.g)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED

Comment 5

8 years ago
>+#if !(defined(OS_MACOSX) || defined(OS_WIN) || (defined(CHROMIUM_MOZILLA_BUILD) && defined(OS_LINUX) defined(ARCH_CPU_64_BITS)))

This line is certainly not going to work as intened; look at the last two conditions.
(Assignee)

Updated

8 years ago
Attachment #499612 - Attachment is obsolete: true
Attachment #499612 - Flags: review?(jones.chris.g)
(Assignee)

Comment 6

8 years ago
Created attachment 507805 [details] [diff] [review]
fix
Attachment #507805 - Flags: review?(jones.chris.g)
Comment on attachment 507805 [details] [diff] [review]
fix

We already overcame this obstacle with mixed-architecture mac OOPP.

  bool WriteIntPtr(intptr_t value) {
    // Always written as a 64-bit value since the size for this type can
    // differ between architectures.
    return WriteInt64(int64(value));
  }

// Always written as a 64-bit value since the size for this type can
// differ between architectures.
bool Pickle::ReadIntPtr(void** iter, intptr_t* result) const {
Attachment #507805 - Flags: review?(jones.chris.g) → review-
(Assignee)

Comment 8

8 years ago
On Mac OS X, (In reply to comment #7)
> Comment on attachment 507805 [details] [diff] [review]
> fix
> 
> We already overcame this obstacle with mixed-architecture mac OOPP.

Chris, Do you say that ReadParam(aMsg, aIter, PRUint32) should be extracted to ReadIntPtr()?

struct ParamTraits<size_t> is never defined on Mac OS X.  So this issue isn't on Mac OS X.  On Mac OS X both 32bit and 64bit is extracted it to ReadUInt32().

xul!IPC::ParamTraits<nsACString_internal>::Read() calls ReadParam(aMsg, aIter, PRUint32)) in http://mxr.mozilla.org/mozilla-central/source/ipc/glue/IPCMessageUtils.h?mark=228-229#228.  So on Mac OS X (that can use OOPP) extracts it to ParamTraits<PRUint32>::Read(m, iter, PRUint32).   But on Windows x86, it extracts to ParamTraits<void *>::Read(m, iter, void*)....
(Assignee)

Comment 9

8 years ago
On Windows x86, struct ParamTraits<uint32> uses ReadSize()
Oh, I misunderstood what you were trying to fix.

Yeah, saving a few bytes here sounds fine.
Comment on attachment 507805 [details] [diff] [review]
fix

r=me assuming it's green on tryserver ;).  This code is very fragile.
Attachment #507805 - Flags: review- → review+
(Assignee)

Comment 12

8 years ago
Comment on attachment 507805 [details] [diff] [review]
fix

pass tests on try server
Attachment #507805 - Flags: approval2.0?

Updated

8 years ago
Attachment #507805 - Flags: approval2.0? → approval2.0-
(Assignee)

Updated

8 years ago
Depends on: 610267

Comment 13

8 years ago
http://hg.mozilla.org/mozilla-central/rev/302723303f42
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
No longer depends on: 610267
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.