468002 - nsDebugImpl's Break() passes bogus wide string to windbgdlg.exe

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This was changed in bug 455381 in the mass change to using the W versions of win32 functions.  However, windbgdlg.exe is ours (in xpcom/windbgdlg), and it treats the cmd line as a C string, not a wide string.  We're now converting to a wide string or something before launching in http://hg.mozilla.org/mozilla-central/annotate/f90c51ed3cd2/xpcom/base/nsDebugImpl.cpp#l400 .  I'm actually not sure what's going on, the net result is that all messages come out as chinese characters instead of correct in the abort/quit/debug dialog box.

Marking as blocking, because there's a potential for subtle corruption here.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

that same push changed windbgdlg.cpp to handle wide char strings.

http://hg.mozilla.org/mozilla-central/rev/0f1b8ab7cd8e

http://hg.mozilla.org/mozilla-central/annotate/f90c51ed3cd2/xpcom/windbgdlg/windbgdlg.cpp

Comment 2

[[e-mail]]

Comment on attachment 346946 [details] [diff] [review]
removed nsCommonProcess 

>-    wsprintf(msg,
>-             "%s\n\nClick Abort to exit the Application.\n"
>-             "Click Retry to Debug the Application..\n"
>-             "Click Ignore to continue running the Application.", 
>+    wsprintfW(msg,
>+             L"%s\n\nClick Abort to exit the Application.\n"
>+             L"Click Retry to Debug the Application..\n"
>+             L"Click Ignore to continue running the Application.", 
>              lpszCmdLine);
What a shame lpszCmdLine is still narrow...

Comment 3

[[e-mail]]

Attachment: Option 1: really make everything wide

I'm not sure this works because I've heard that not everyone supports wWinMain.

Comment 4

[[e-mail]]

Attachment: Option 2: reuse lpszCmdLineW

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

I think the switch to wWinMain is preferable since we don't support win95/98 anymore.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

...and thank you for fixing the spacing

Comment 7

[[e-mail]]

Pushed changeset 79c023857355 to mozilla-central.

(Actually it was some hours ago, I just forgot to update the bug until now...)

Comment 8

[[e-mail]]

Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

Needed to fix regression from bug 455381.

Comment 9

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

Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

Blocker, doesn't need approval.

Comment 10

[[e-mail]]

(In reply to comment #9)
> Blocker, doesn't need approval.
Thanks for pointing that out.

Pushed changeset f8aedd7dc4c7 to releases/mozilla1.9.1