Regression with Blizzard patching software

[Aneurin Price]

I recently discovered that the software used by Blizzard to patch Diablo 
2 and Warcraft 3 (and possibly the same version used with other games), 
has stopped working. It starts do do something, but before actually 
applying the patch it crashes out, with the final error message being:

Unhandled exception: page fault on write access to 0x00000000 in 32-bit 
code (0x408ba2e0).
In 32-bit mode.
0x408ba2e0 (DIALOG_CreateIndirect+0x300 [dialog.c:636] in USER32.DLL): 
movl    $0x0,0x0(%eax)
139             return TRUE;
Wine-dbg>WineDbg terminated on pid a

The whole of the scrollback buffer is filled with the usual messages 
about loading debug information. I could run it with script to see 
what's happening before that, but I don't think it's really necessary:
Since this *used* to work, I've gone through some CVS regression testing 
and determined that it stopped working sometime between 2004-04-19 
22:00:00 and 2004-04-19 22:03:00 CST, so I'm willing to bet that this is 
what changed: http://www.winehq.org/hypermail/wine-cvs/2004/04/0272.html
Since I have no understanding of what's going on here I really coudn't 
suggest a fix, but I'm happy to provide any further information I can 
and thought this would be worth pointing out.
Aneurin Price

[Robert Shearman]

Aneurin Price wrote
> I recently discovered that the software used by Blizzard to patch Diablo
> 2 and Warcraft 3 (and possibly the same version used with other games),
> has stopped working. It starts do do something, but before actually
> applying the patch it crashes out, with the final error message being:
>
> Unhandled exception: page fault on write access to 0x00000000 in 32-bit
> code (0x408ba2e0).
> In 32-bit mode.
> 0x408ba2e0 (DIALOG_CreateIndirect+0x300 [dialog.c:636] in USER32.DLL):
> movl    $0x0,0x0(%eax)
> 139             return TRUE;
> Wine-dbg>WineDbg terminated on pid a
>
> The whole of the scrollback buffer is filled with the usual messages
> about loading debug information. I could run it with script to see
> what's happening before that, but I don't think it's really necessary:
> Since this *used* to work, I've gone through some CVS regression testing
> and determined that it stopped working sometime between 2004-04-19
> 22:00:00 and 2004-04-19 22:03:00 CST, so I'm willing to bet that this is
> what changed: http://www.winehq.org/hypermail/wine-cvs/2004/04/0272.html
> Since I have no understanding of what's going on here I really coudn't
> suggest a fix, but I'm happy to provide any further information I can
> and thought this would be worth pointing out.
> Aneurin Price

I must compliment you on excellent debugging. It is rare that we get a debug
report as good as this, with a specific path that broke something!
I believe the correct fix is to change DEFDLG_InitDlgInfo (defdlg.c) to not
be static and then to call it instead of DIALOG_get_info on line 636 of
dialog.c
I don't have much time at the moment, so could someone check this is right
and if so submit a patch?
One thing that I don't understand is why the default dialog window proc
isn't getting called at all, unless the app is using an ugly hack for some
reason, like hooks. Perhaps more debugging is needed in DEFDLG_InitDlgInfo?

Rob

[Abby Ricart]

> <>I believe the correct fix is to change DEFDLG_InitDlgInfo (defdlg.c) 
> to not be static and then to call it instead of DIALOG_get_info on 
> line 636 of dialog.c I don't have much time at the moment, so could 
> someone check this is right and if so submit a patch?
> <>Rob

Since DEFDLG_InitDlgInfo calls DIALOG_get_info internally wouldn't it 
also return a NULL pointer rather than a DIALOGINFO* ? Also, Isn't 
DEFDLG_InitDlgInfo out of scope for dialog.c?

(Disclaimer: I'm not a WINE developer. Matter fact, I'm not a developer 
at all. WINE is the first project I've ever attempted to work on)

[Saulius Krasuckas]

That seems to be reported at wine-bugs:

http://bugs.winehq.org/show_bug.cgi?id=2210

On Thu, 20 May 2004, Abby Ricart wrote:
> > <>I believe the correct fix is to change DEFDLG_InitDlgInfo (defdlg.c) 
> > to not be static and then to call it instead of DIALOG_get_info on

Did you really meant "instead?  Why not "before"?
> > line 636 of dialog.c I don't have much time at the moment, so could 
> > someone check this is right and if so submit a patch?
> > <>Rob
>
> Since DEFDLG_InitDlgInfo calls DIALOG_get_info internally wouldn't it
> also return a NULL pointer rather than a DIALOGINFO* ?

AFAICS, it really would return NULL for the first time.  In this case
following code will be execed:

C    if(!dlgInfo)
C    {
C        if (!(dlgInfo = HeapAlloc( GetProcessHeap(), 0, sizeof(*dlgInfo) ))) return NULL;

and all should be OK.  But DEFDLG_InitDlgInfo() doesn't get called.  

In my case the reason of crash may be custom dlgProc set during this:
DIALOG_CreateIndirect+0x24e(hInst=0x400000, dlgTemplate=0x4065b8, owner=0x0, dlgProc=0x401260, param=0x0, unicode=0x0, modal=0x0) [dialog.c:636]

while that dlgProc can be unaware of things wine needs to be set up, like 
a DWL_WINE_DIALOGINFO:

C inline static DIALOGINFO *DIALOG_get_info( HWND hwnd )
C {
C     return (DIALOGINFO *)GetWindowLongW( hwnd, DWL_WINE_DIALOGINFO );
C }

In another case (reported at wine-bugs) custom dlgProc isn't set:
DIALOG_CreateIndirect+0x300(hInst=0x400000, dlgTemplate=0x56c5c4, owner=0x0, dlgProc=0x0, param=0x0, unicode=0x0, modal=0x0) [dialog.c:636]

Still crash waits us in this case.  Something is wrong with handling
WM_CREATE or WM_INITDIALOG messages?
> Also, Isn't DEFDLG_InitDlgInfo out of scope for dialog.c?

AFAIK it is.  Maybe that's why Rob meant it "to change to not be static" ?
> (Disclaimer: I'm not a WINE developer. Matter fact, I'm not a developer 
> at all. WINE is the first project I've ever attempted to work on)

And so am I.  ;-)