Closed Bug 646534 Opened 9 years ago Closed 6 years ago

crash [@ _atof_l] under nsCRLManager::ImportCrl

Categories

(Core Graveyard :: Security: UI, defect, critical)

x86
Windows 7
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: timeless, Assigned: timeless)

References

Details

(Keywords: crash, Whiteboard: [sg:dos])

Crash Data

Attachments

(7 files, 2 obsolete files)

Signature	_atof_l
UUID	c163c396-0241-47ad-9c33-7b0002110330
Time 	2011-03-30 02:14:17.206590
Uptime	14
Last Crash	56 seconds before submission
Install Age	32857 seconds (9.1 hours) since version was first installed.
Product	Firefox
Version	4.2a1pre
Build ID	20110329030437
Branch	2.2
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
CPU	x86
CPU Info	GenuineIntel family 6 model 28 stepping 10
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x1
User Comments	
App Notes 	AdapterVendorID: 8086, AdapterDeviceID: a011, AdapterDriverVersion: 8.14.10.2230
D3D10 Layers? D3D10 Layers-
D3D9 Layers? D3D9 Layers-
Processor Notes 	WARNING: No 'client_crash_date' could be determined from the Json file
EMCheckCompatibility	False
Bugzilla - Report this Crash
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	mozcrt19.dll 	_atof_l 	obj-firefox/memory/jemalloc/crtsrc/atof.c:59
1 	mozcrt19.dll 	atof 	obj-firefox/memory/jemalloc/crtsrc/atof.c:71
2 	xul.dll 	nsCRLManager::ImportCrl 	security/manager/ssl/src/nsCRLManager.cpp:262
3 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4 	xul.dll 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1613
5 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:4799
6 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:740
7 	mozjs.dll 	js::ExternalInvoke 	js/src/jsinterp.cpp:863
8 	mozjs.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:5173
9 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1672
10 	xul.dll 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:588
11 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
12 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
13 	xul.dll 	nsStreamLoader::OnStopRequest 	netwerk/base/src/nsStreamLoader.cpp:125

This is basically because the nsCRLManager code doesn't use the pref service properly. I have a patch series which tries to rewrite this. I'll post it shortly.
this belongs to bug 577266, but it's part of my series and thus it's best to have it pushed from here.
Attachment #523914 - Flags: review?(kaie)
this is just comment cleanup
Attachment #523915 - Flags: review?(kaie)
while nss uses goto's, c++ really shouldn't.
Attachment #523916 - Flags: review?(kaie)
Attachment #523917 - Flags: review?(kaie)
this fixes the bug
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #523918 - Flags: review?(kaie)
Blocks: 645819
Attachment #523914 - Flags: review?(kaie) → review+
Attachment #523915 - Flags: review?(kaie) → review+
Attachment #523916 - Flags: review?(kaie) → review+
Keywords: checkin-needed
Whiteboard: [please push as many reviewed things as possible]
Whiteboard: [please push as many reviewed things as possible] → [sg:dos][please push as many reviewed things as possible]
http://hg.mozilla.org/mozilla-central/rev/ea6a053c57b0
http://hg.mozilla.org/mozilla-central/rev/0dcdf1c5a7fa

Brian, do you think you can take over the review on the remaining patches here, please?  Thanks!
Keywords: checkin-needed
Whiteboard: [sg:dos][please push as many reviewed things as possible] → [sg:dos]
Comment on attachment 523917 [details] [diff] [review]
cleanup whitespace

r=kaie
Attachment #523917 - Flags: review?(kaie) → review+
Comment on attachment 523918 [details] [diff] [review]
fix pref handling, refactor, avoid manual string management

This patch has too many changes.
It's too difficult to review it.
I don't know how I am supposed to guarantee that your changes doesn't introduce semantic changes.

Why didn't you simply fix the crash first?
Comment on attachment 523918 [details] [diff] [review]
fix pref handling, refactor, avoid manual string management

r-

I'm trying to morph it into changes that result in a readable patch.
Attachment #523918 - Flags: review?(kaie) → review-
Comment on attachment 523918 [details] [diff] [review]
fix pref handling, refactor, avoid manual string management

This patch changes error result handling in several scenarios
Timeless, please, it's good that you're trying to fix bugs, but it's really unproductive if you produce unreadable patches. I already took me more than 3 hours to un-refactor your patch and trying to get it into a state that is readable.

I'm really sure it was less work for you to write the initial patch.
Attached patch Part 5 patch v2 (obsolete) — Splinter Review
Attachment #525904 - Flags: review?(bsmith)
Attached patch Part 5 patch v3Splinter Review
Attachment #525902 - Attachment is obsolete: true
Attachment #525904 - Attachment is obsolete: true
Attachment #525904 - Flags: review?(bsmith)
Attachment #525906 - Flags: review?(bsmith)
Crash Signature: [@ _atof_l]
This is a crash bug and should get some attention.

I spent hours on this already, I'd appreciate some help.

But I have no idea whether this crash is really reproducible or if the patch even still applies.
This was fixed by removing the feature. See bug 867465.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Depends on: 867465
Resolution: --- → INVALID
Comment on attachment 525906 [details] [diff] [review]
Part 5 patch v3 - ignoring whitespace

The feature was removed so I am dropping this review request.
Attachment #525906 - Flags: review?(brian)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.