nsMemoryImpl::Alloc (0) in XPCConvert::JSArray2Native

RESOLVED FIXED

Status

()

Core
XPConnect
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({assertion, fixed-aviary1.0, fixed1.7})

Trunk
x86
Windows XP
assertion, fixed-aviary1.0, fixed1.7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Assignee)

Description

14 years ago
xpcom.dll!nsMemory::Alloc(unsigned int size=0)  Line 88	C++
>	xpc3250.dll!XPCConvert::JSArray2Native(XPCCallContext & ccx={...}, void * *
d=0x0012f234, long s=1, unsigned int count=0, unsigned int capacity=0, const
nsXPTType & type={...}, int useAllocator=0, const nsID * iid=0x0012f348,
unsigned int * pErr=0x0012f398)  Line 1735 + 0x11	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...},
XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 1981 + 0x21	C++

###!!! ASSERTION: nsMemoryImpl::Alloc of 0: 'size', file
mozilla/xpcom/base/nsMemoryImpl.cpp, line 324
Break: at file mozilla/xpcom/base/nsMemoryImpl.cpp, line 324
0 [native frame]
1 sendCallbackString(message = "")
["file:///C:/DBG/bin/components/hs_Trouble_Maker.js":867]
    totalBytes = undefined
    bytesWritten = undefined
    this = [object Object]

const sockService_ =
  Components.classes["@mozilla.org/network/socket-transport-service;1"].
  getService().QueryInterface(Components.interfaces.nsISocketTransportService);
//867:
      this.connection_ = sockService_.createTransport([], 0, "localhost",
                                                      this.port_, null);

code that causes the problem:
#define POPULATE(_mode, _t)                                                  \
    PR_BEGIN_MACRO                                                           \
        cleanupMode = _mode;                                                 \
        if(nsnull == (array = nsMemory::Alloc(capacity * sizeof(_t))))       \
        {                                                                    \
            if(pErr)                                                         \
                *pErr = NS_ERROR_OUT_OF_MEMORY;                              \
            goto failure;                                                    \
        }                                                                    \
        for(initedCount = 0; initedCount < count; initedCount++)             \
        {                                                                    \
            if(!JS_GetElement(cx, jsarray, initedCount, &current) ||         \
               !JSData2Native(ccx, ((_t*)array)+initedCount, current, type,  \
                              useAllocator, iid, pErr))                      \
                goto failure;                                                \
        }                                                                    \
    PR_END_MACRO

    case nsXPTType::T_CHAR_STR      : POPULATE(fr, char*);          break;

locals:
+	ccx	{mRefCnt={mValue=0 } _mOwningThread={mThread=0x00426080 }
mState=READY_TO_CALL ...}	XPCCallContext &
	d	0x0012f234	void * *
	s	1	long
	count	0	unsigned int
	capacity	0	unsigned int
+	type	{...}	const nsXPTType &
	useAllocator	0	int
+	iid	0x0012f348 {m0=3059557040 m1=7633 m2=4530 ...}	const nsID *
+	pErr	0x0012f398	unsigned int *
	array	0x00000000	void *
	cleanupMode	fr	XPCConvert::JSArray2Native::__l7::CleanupMode
	len	0	unsigned long
+	cx	0x00c40498 {links={next=0x01c2a6d0 {next=0x01cd6460 {next=0x01d097c8
prev=0x01c2a6d0 } prev=0x00c40498 {next=0x01c2a6d0 prev=0x00b67400 } }
prev=0x00b67400 {next=0x00c40498 {next=0x01c2a6d0 prev=0x00b67400 }
prev=0x00b64218 {next=0x00b67400 prev=0x02a945f8 } } } interpLevel=1
stackLimit=0 ...}	JSContext *
+	jsarray	0x02c99790 {map=0x02cb0c68 {nrefs=1 ops=0x00e08160 _js_ObjectOps
nslots=5 ...} slots=0x02c82934 }	JSObject *
	initedCount	1242072	unsigned int
	current	1	long

Some code in this function tolerates 0 capacity, some code (like the code here)
does not.
(Assignee)

Comment 1

14 years ago
Another testcase (much more pathological)

SoapCall=Components.Constructor("@mozilla.org/xmlextras/soap/call;1");
SoapParam=Components.Constructor("@mozilla.org/xmlextras/soap/parameter;1");
SOAPParameter=function SOAPParameter(){
  var args=["value", "name", "namespace", "schemaType", "encoding"];
  var soapParam = new SoapParam;
  for (var i = 0; i<arguments.length; ++i)
    soapParam[args[i]]=arguments[i];
  return soapParam;
}
SOAPCall=function SOAPCall(){
  var args=[];
  var soapCall = new SoapCall;
  for (var i = 0; i<arguments.length; ++i)
    soapCall[args[i]]=arguments[i];
  return soapCall;
}
  var soapversion = 0;  //  Version 1.1

  var method = "GetLastTradePrice";

  var object = "uri:some-namespace";

  var headers = new Array(
    );
  
  var params = new Array(
    new SOAPParameter("a string", "foo"),
    new SOAPParameter(4, "bar"),
    new SOAPParameter(true, "baz"),
    new SOAPParameter(new Array(3, "another string", false, 5.235)),
    new SOAPParameter({name: "Fido", breed: "Lab", color: "Black"})
    );

  var s = new SOAPCall();
  s.encode(soapversion, method, object, headers.length, headers, params.length,
params);
Status: NEW → ASSIGNED
(Assignee)

Comment 2

14 years ago
Created attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

This matches the behavior of XPCVariant::InitializeData
(Assignee)

Updated

14 years ago
Attachment #146277 - Flags: superreview?(brendan)
Attachment #146277 - Flags: review?(BradleyJunk)
I'll wait for dbradley to r=, but looks good so far.  Any other such cases?

/be

Comment 4

14 years ago
Comment on attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

r=dbradley

If we're going to follow this pattern and passing in zero length arrays to
JSArray2Native is going to be "bad" then we should put in an assert stating
this.

I'd probably have chosen to handle this within, but it could get messy, as
JSArray2Native may not have enough knowledge to know what the caller is wanting
for empty arrays.

So this fixes it, but I'm not sure it's ideal. We can deal with that later.
Attachment #146277 - Flags: review?(BradleyJunk) → review+
(Assignee)

Updated

14 years ago
Keywords: assertion
Summary: nsMemoryImpl::Alloc in XPCConvert::JSArray2Native → nsMemoryImpl::Alloc (0) in XPCConvert::JSArray2Native
Comment on attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

rs=brendan@mozilla.org.

/be
Attachment #146277 - Flags: superreview?(brendan) → superreview+

Comment 6

14 years ago
Comment on attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

Can we get this on the branch?	Using the JS debugger on windows debug builds
(which people basing products off the branch are likely to do) is incredibly
irritating with this dialog box popping up constantly.
Attachment #146277 - Flags: approval1.7?

Comment 7

14 years ago
Comment on attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #146277 - Flags: approval1.7? → approval1.7+
(Assignee)

Updated

14 years ago
Keywords: fixed1.7
Whiteboard: fixed-aviary1.0
(Assignee)

Comment 8

14 years ago
Comment on attachment 146277 [details] [diff] [review]
only convert array if it has elements. otherwise dp->val.p is null, which is fine

mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp	1.77
mozilla/js/src/xpconnect/src/xpcwrappednative.cpp	1.84

mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp	1.76.14.1
mozilla/js/src/xpconnect/src/xpcwrappednative.cpp	1.82.2.1
Attachment #146277 - Attachment is obsolete: true
Why is this open? Comment 8 indicates a trunk check-in, and the trunk code
appears to have the patch. Then again the patch is marked obsolete... what's
remaining here?
Keywords: fixed-aviary1.0
Whiteboard: fixed-aviary1.0
QA Contact: pschwartau → xpconnect

Comment 10

11 years ago
timeless,
ping regarding comment 9 

Comment 11

11 years ago
Did it get checked into the 1.7 branch. That's the only thing I see potentially outstanding. Otherwise I'd say mark it fixed.

Comment 12

10 years ago
at this point if it didn't check into 1.7 it's no loss
=> fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.