[Bug 12902] New: IHP Kitchen: Unhandled page fault on read access when starting

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

Summary: IHP Kitchen: Unhandled page fault on read access when
                    starting
           Product: Wine
           Version: 0.9.60
          Platform: PC
               URL: http://www.ikea.com/ms/en_CA/rooms_ideas/kitchen/IHP_Kit
                    chen.EXE
        OS/Version: Linux
            Status: NEW
          Keywords: download, regression
          Severity: normal
          Priority: P2
         Component: -unknown
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
            Blocks: 4935

Created an attachment (id=12606)
 --> (http://bugs.winehq.org/attachment.cgi?id=12606)
+relay,+seh,+tid log in git, bzip2 -9'ed

IKEA Kitchen Planner crashes on startup. This is a regression, though not sure
when it started...I'll try to run a regression test. +relay,+seh,+tid attached.

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

Marcus Meissner  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

--- Comment #1 from Anastasius Focht   2008-05-02 04:01:20 ---
Hello,

no need for regression testing .. I'm faster debugging it than you do
regression testing ;-)

There is a RegisterClassExA call in app code which looks pretty brain damaged.
The app initializes the stack (structures) with 0xCCCCCCCC before writing to
structure members.

The first call to RegisterClassA() succeeds, lpWndClass members (stack dump)
before the call:

--- snip lpWndClass ---
0033F8E0   00000000 ; style
0033F8E4   00401C80 ; lpfnWndProc
0033F8E8   00000000 ; cbClsExtra
0033F8EC   00000000 ; cbWndExtra
0033F8F0   00400000 ; hInstance
0033F8F4   00001126 ; hIcon
0033F8F8   000010DE ; hCursor
0033F8FC   00000000 ; hbrBackground
0033F900   005E2000 ; lpszMenuName -> "Main"
0033F904   005E4BA5 ; lpszClassName -> "Furnish Pro"
--- snip lpWndClass ---

The next call immediately following is RegisterClassExA().
Note: only the cbSize member is explicitly initialized!

WNDCLASSEXA* = 0x0033F908

--- snip ---
0033F908   00000030 ; cbSize (+0x0)
0033F90C   CCCCCCCC ; style (+0x4)
0033F910   CCCCCCCC ; lpfnWndProc (+0x8)
0033F914   CCCCCCCC ; cbClsExtra (+0xC)
0033F918   CCCCCCCC ; cbWndExtra (+0x10)
0033F91C   CCCCCCCC ; hInstance (+0x14)
0033F920   CCCCCCCC ; hIcon
0033F924   CCCCCCCC ; hCursor
0033F928   CCCCCCCC ; hbrBackground
0033F92C   CCCCCCCC ; lpszMenuName
0033F930   CCCCCCCC ; lpszClassName
0033F934   00001126 ; hIconSm
--- snip ---

This ought to fail in Windows with last error ERROR_INVALID_PARAMETER.
After that call, the app initializes all other struct members and
RegisterClassExA is called again, now succeeding.

The offending code in wine user32:

--- snip dlls/user32/class.c ---
ATOM WINAPI RegisterClassExA( const WNDCLASSEXA* wc ) 
{ 
..
    if (wc->hInstance == user32_module)
    {
        /* we can't register a class for user32 */
        SetLastError( ERROR_INVALID_PARAMETER );
        return 0;
    }
    if (!(instance = wc->hInstance)) instance = GetModuleHandleW( NULL );

if (!IS_INTRESOURCE(wc->lpszClassName))
    {
        WCHAR name[MAX_ATOM_LEN + 1];

if (!MultiByteToWideChar( CP_ACP, 0, wc->lpszClassName, -1, name,
MAX_ATOM_LEN + 1 )) return 0;
        classPtr = CLASS_RegisterClass( name, instance, !(wc->style &
CS_GLOBALCLASS),
                                        wc->style, wc->cbClsExtra,
wc->cbWndExtra );
    }
    else
    {
        classPtr = CLASS_RegisterClass( (LPCWSTR)wc->lpszClassName, instance,
                                        !(wc->style & CS_GLOBALCLASS),
wc->style,
                                        wc->cbClsExtra, wc->cbWndExtra );
    }
    if (!classPtr) return 0; 
...
}
--- snip dlls/user32/class.c ---

It crashes by accessing pointer style members - not expecting garbage ;-)
Same applies to "W" version.

Interestingly this application bug doesn't trigger page fault in windows.
The culprit is probably the order of structure member evaluation.

"classExtra" and "winExtra" are possible non-pointer candidates to work around
this problem without introducing another (unnecessary) SEH guard.
Unfortunately these members are checked at relatively late stage in wine
(CLASS_RegisterClass).
If you move the code into appropriate functions, this app bug is worked around
(yes, code small duplication but well).

--- snip example fix ---
diff --git a/dlls/user32/class.c b/dlls/user32/class.c
index ff0e1f8..d39350c 100644
--- a/dlls/user32/class.c
+++ b/dlls/user32/class.c
@@ -526,6 +526,13 @@ ATOM WINAPI RegisterClassExA( const WNDCLASSEXA* wc )
     CLASS *classPtr;
     HINSTANCE instance;

+    /* Fix the extra bytes value */
+    if (wc->cbClsExtra < 0 || wc->cbWndExtra < 0)
+    {
+         SetLastError( ERROR_INVALID_PARAMETER );
+         return NULL;
+    }
+
     if (wc->hInstance == user32_module)
--- snip example fix ---

The app then starts loading, displaying main windows and splash screen and then
crashes again due other wine bugs.
While looking at the generated stack frame code it seems the compiler used 0xCC
CCCCCC initialization values for all stack frame members.
Thus other crashes are probably related to this fact.

If you need help for other crashes, call me again .. but fix this one first.

Regards

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Nikolay Sivov   2008-05-10 12:23:32 ---
(In reply to comment #1)
> diff --git a/dlls/user32/class.c b/dlls/user32/class.c
> index ff0e1f8..d39350c 100644
> --- a/dlls/user32/class.c
> +++ b/dlls/user32/class.c
> @@ -526,6 +526,13 @@ ATOM WINAPI RegisterClassExA( const WNDCLASSEXA* wc )
>      CLASS *classPtr;
>      HINSTANCE instance;
> - /* Fix the extra bytes value */
> - if (wc->cbClsExtra < 0 || wc->cbWndExtra < 0)
> - {
> -
>     SetLastError( ERROR_INVALID_PARAMETER );
> -
>     return NULL;
> - }
> - if (wc->hInstance == user32_module)

This just hide actual bug I think. cbClsExtra and cbWndExtra are checked later
in CLASS_RegisterClass. I think the actual problem is in hInstance which isn't
null in this case (0xCCCCCCCC) so treated as valid but it isn't. Is there any
way to check it properly? Such as get a list of current instance handles for
current process (RegisterClass works only in calling process scope) and that
passed handle is in it..or something similar

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

--- Comment #3 from Alexandre Julliard   2008-05-19 16:28:11 ---
I committed the fix. It goes a little further and then crashes because it
expects scrollbars to have an ASCII window proc.

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #4 from Alexandre Julliard   2008-05-21 05:24:14 ---
I put in a fix for the scrollbar thing. It starts fine now.

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #5 from Alexandre Julliard   2008-05-23 10:36:46 ---
Closing bugs fixed in 1.0-rc2.

[Bug 12902] IHP Kitchen: Unhandled page fault on read access when starting

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

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |c362081a791632b88c96405cb66
                   |                            |ae6347d391fae
                 CC|                            |[e-mail]
          Component|-unknown                    |user32