436531 - asmXPTC_InvokeByIndex Bogus Return Value in WinMobile builds

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[John Wolfe (:wolfe)]

When calling XPTC_InvokeByIndex on WinMobile builds, the return value of the function is a random, bogus value.

The function prototype for XPTC_InvokeByIndex is:

extern "C" nsresult
asmXPTC_InvokeByIndex( nsISupports* that,
                       PRUint32 methodIndex,
                       PRUint32 paramCount,
                       nsXPTCVariant* params,
                       PRUint32 pfn_CopyToStack,
                       PRUint32 pfn_CountWords);  


This nsresult random return value will mess up anyone who is examining the function's return code.  

All arguments passed into the function appear to be handled correctly.

Patch for this bug will also update the XPCOM/Proxy/Tests to check for this (and other) potential error.

Comment 1

[John Wolfe (:wolfe)]

Attachment: WinCE XPCOM Patch

Revises XPCOM/proxy/tests to be more complete, fixes asmXPTC_InvokeByIndex return code

Comment 2

[John Wolfe (:wolfe)]

Attachment: Update of previous patch - more info output directly via printf

This is a revised patch for the WinCE XPCOM return code value being incorrect.

This patch adds more duplicate output via printf and LOG(), allowing a casual tester to see immediate feedback about the XPCOM proxy functionality.

Comment 3

[John Wolfe (:wolfe)]

Attachment: Oops - Added a needed IDL file

HG does not take added files unless you actually do an "hg add" -- my oops.

Here is the revised patch file, which should compile XPCOM/proxy/tests executables correctly.

Comment 4

[John Wolfe (:wolfe)]

Attachment: Two more files added to the patch

I pulled out the large Proxy Test class into its own file - then end up including that CPP file back into proxytests.cpp because of automated build Makefile struggles.  This latest patch has all files added into the mix - with the result compiling the <OBJDIR>/XPCOM/proxy/tests directory.

Comment 5

[John Wolfe (:wolfe)]

Attachment: Removed two commented out assembly code lines

Just removed two unnecessary commented out lines of assembly code which would have cluttered up the ASM file.  As per request of DougT

Comment 6

[Benjamin Smedberg]

Comment on attachment 324695 [details] [diff] [review]
Removed two commented out assembly code lines

>diff -r 41ccf537b240 build/wince/shunt/build/vs8/mozce_shunt_static.vcproj

I have not reviewed this file, as I don't know what it's for.

>diff -r 41ccf537b240 xpcom/proxy/tests/Makefile.in

>+CPP_UNIT_TESTS += proxytests.cpp \
>+		proxy-create-threadsafety.cpp \
>+		$(NULL)
>+XPIDLSRCS	= nsITestProxy.idl \
>+                  nsITestProxyOrig.idl

please indent both of these as
CPP_UNIT_TESTS += \
  file1.cpp \
  file2.cpp \
  $(NULL)

with no tabs

Comment 7

[John Wolfe (:wolfe)]

Attachment: Benjamin's requested change to Makefile.in

Same as Benjamin's reviewed file, but with different indentation for Makefile.in, and removed mozce_shunt_static.vcproj and nsProxyEvent.cpp file changes.

The mozce_shunt_static.vcproj file change and the nsProxyEvent.cpp file change should not have been included in the patch to be reviewed. 

Thanks to Benjamin for catching those un-needed patch hunks!

Comment 8

[John Wolfe (:wolfe)]

Added keyword checkin-needed

Comment 9

[John Wolfe (:wolfe)]

Added Checkin-needed flag.

Comment 10

[Reed Loden [:reed]]

Pushed as 17135:2e3d61018e3d.

Comment 11

[Dave Camp (:dcamp)]

This was backed out due to build failures.  The Makefile was broken (trailing spaces after some \'s, missing $(NULL) in XPIDLSRCS), and a header file was missing.

Could you post a fixed patch please?

Comment 12

[John Wolfe (:wolfe)]

Attachment: Fixed Patch File - Compiles against my just-pulled mozilla-central repository

Fixed patch file, including bugs listed and missing header/source files.

Compiles on my pulled-this-morning (7-Sep-08) mozilla-central.  I apologize for creating the extra work for you, Reed and Dave.

Comment 13

[John Wolfe (:wolfe)]

Comment on attachment 337352 [details] [diff] [review]
Fixed Patch File - Compiles against my just-pulled mozilla-central repository

Sent back to Benjamin for re-review.  Nothing new in this patch, just missing files and Makefile.in fixes (I have no idea why/where the previous patch came from in my compiling system).

-- John Wolfe ([e-mail])

Comment 14

[:Gavin Sharp [email: [e-mail]]]

You want to request review from ":bs" (or benjamin@smedbergs) instead of that address (he only uses it for watching).

Comment 15

[John Wolfe (:wolfe)]

Comment on attachment 337352 [details] [diff] [review]
Fixed Patch File - Compiles against my just-pulled mozilla-central repository

Thanks to Gavin for pointing out that Benjamin uses that gmail account for watching bugs, and should be requested via a different email.

Comment 16

[John Wolfe (:wolfe)]

Added checkin-needed flag.

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> Added checkin-needed flag.

Multiple failurse to apply: needs unbitrotting.

Comment 18

[John Wolfe (:wolfe)]

Attachment: v.8 patch

Same as Attachment 337352 [details] [diff], but adjusts UUIDs so each are unique in XPCOM Proxy Tests.

Not for review yet, want to make sure this does not mess up the Win32 build.

Also, for some reason, GetProxyForObject is returning NULL proxy objects under WinMobile 6 XPCOM Proxy Test (running on HTC Touch Diamond USA).

Comment 19

[Doug Turner (:dougt)]

just so it is obvious what I am doing - the patch is the same as what was review by bsmedberg.  wolfe updated it and changed the uuids as requested.  I looked over it, applied it to my build and it works on windows ce and win 32.

Comment 20

[Doug Turner (:dougt)]

Comment on attachment 342100 [details] [diff] [review]
v.8 patch

this patch doesn't work on non-windows machines.  Also, it is missing the new files from the previous patch.

Comment 21

[Doug Turner (:dougt)]

Attachment: just the wince specific asm

Comment 22

[Doug Turner (:dougt)]

wince arm changes checked in:

[e-mail]
Wed Oct 08 20:58:40 2008 -0700	7e5d581bc86d	wolfe — Bug 436531 - asmXPTC_InvokeByIndex Bogus Return Value in WinMobile builds. r=bsmedberg/dougt

Wolfe, please create a separate bug for just the tests cases.