Jon Henry Reporter
	Description • 21 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031211 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031211 Firebird/0.7+

I noticed this while working on a patch for Firebird bug 210043. Seemingly only
in builds done with GCC on windows, Mozilla will crash after trying to quit if
you have copied an image to the clipboard using either the patch to the bug I
mentioned or the Copy Image extension. MozillaZine readers did some testing and
confirmed that it does not happen in MSVC builds, so I'm not sure if this is
technically a Mozilla bug that MSVC somehow gets around or a bug in GCC itself.

Reproducible: Always

Steps to Reproduce:
1. Create a build using GCC on Win32.
2. Copy an image to the clipboard in Mozilla.
3. Exit the browser.

Actual Results:  
After the last Mozilla window disappears, a dialog indicating Mozilla has
crashed comes up.

Expected Results:  
Quit without incident.

GCC reports itself as "gcc version 3.3.1 (mingw special 20030804-1)" on my system.

	Jon Henry Reporter
	Comment 2 • 21 years ago

In my testing this fixed the crash without any discernable side effects.
However, I admit I don't completely grok the chain of calls that leads to this
code executing with bogus values. But it seems reasonable to me to return NULL
here rather than entering GetText with data that will definitely crash.

	Brian Ryner (not reading)
	Comment 3 • 21 years ago

I don't know this code well enough to say whether this fix is correct.  Maybe
Dan M does.

	Dan M
	Comment 4 • 21 years ago

I don't know this code at all. But my guess is there are other text formats out
there other than the two well-known Windows clipboard formats already taken care
of in this method (CF_TEXT and CF_UNICODETEXT) (text/html or
text/tab-separated-values, say? i don't know if they count as CF_TEXT) that
chopping this method off right here will cause to go missing. I'm thinking the
downstream text converters should be armoured up, instead. Ask Pink?

	Brian Ryner (not reading)
	Comment 5 • 21 years ago

Comment on attachment 137309 [details] [diff] [review]
Possible patch

I agree with dan, I'm not really comfortable with this at the moment.  Someone
needs to step through it in the debugger and see what arguments are being
passed to memcpy that cause the crash.

	Jon Henry Reporter
	Comment 6 • 21 years ago

Comment on attachment 137309 [details] [diff] [review]
Possible patch

Canceling my review request to ere, since this needs more work anyway.

	John Morrison
	Comment 7 • 21 years ago

> #1  0x08e26118 in nsDataObj::GetText(nsACString const&, tagFORMATETC&, 
> tagSTGMEDIUM&) (this=0xe860c28, aDataFlavor=@0xe8610f8, aFE=@0x22fc54, 
> aSTG=@0x22fc28)
>    at c:/Mozillaroot/mozilla/widget/src/windows/nsDataObj.cpp:750
>	dest = 0x25eaf0 "\rЁн║\230\001ллллллллю■"
>	source = 0xc1d2 <Address 0xc1d2 out of bounds>
>	status = 0
>	data = (void *) 0xc1d2
> ...

> Someone needs to step through it in the debugger and see what arguments are 
> being passed to memcpy that cause the crash.

Well, source is bogus. So, where does data come from?

	Robert Kaiser
	Comment 8 • 12 years ago

This bug has no information on current versions and no clear lead as to what the crashes that it tracks are. This should either be updated with current info and reopened or new bugs be filed on concrete actions/crashes on current code.

(Even though this has a patch from back then, the whole toolchain and code involved here has changed so much that this is very likely to be bogus nowadays - and I'm not even sure if we support MinGW gcc at this time.)