[PATCH] gdi32: Accept 16-bit handles in get_dc_attr().

[Oleh Nykyforchyn]

When background is erased in a 16 bit app, a handle to DC is passed
through USER16 functions, namely CallWindowProc16 (USER.122) and
DefDriverProc16 (USER.255), whose respective parameters are of type
WPARAM16, hence the upper part of the handle is cut. Later
the returned handle is rejected by get_dc_attr() in GDI32 because
its type is 0, although such handles are accepted by handle_entry().
This results in black and not cleared background.  Allowing type==0
in get_dc_attr() solves the problem.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51899
Signed-off-by: Oleh Nykyforchyn 
---
 dlls/gdi32/dc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c
index 1511c565194..a161b3cb640 100644
--- a/dlls/gdi32/dc.c
+++ b/dlls/gdi32/dc.c
@@ -56,7 +56,7 @@ DC_ATTR *get_dc_attr( HDC hdc )
 {
     DWORD type = gdi_handle_type( hdc );
     DC_ATTR *dc_attr;
-    if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
+    if ((type && (type & 0x1f0000) != NTGDI_OBJ_DC) || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
     {
         SetLastError( ERROR_INVALID_HANDLE );
         return NULL;

[Nikolay Sivov]

On 10/23/21 10:34 AM, Oleh Nykyforchyn wrote:
> When background is erased in a 16 bit app, a handle to DC is passed
> through USER16 functions, namely CallWindowProc16 (USER.122) and
> DefDriverProc16 (USER.255), whose respective parameters are of type
> WPARAM16, hence the upper part of the handle is cut. Later
> the returned handle is rejected by get_dc_attr() in GDI32 because
> its type is 0, although such handles are accepted by handle_entry().
> This results in black and not cleared background.  Allowing type==0
> in get_dc_attr() solves the problem.
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51899
> Signed-off-by: Oleh Nykyforchyn
>
> dlls/gdi32/dc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c
> index 1511c565194..a161b3cb640 100644
> --- a/dlls/gdi32/dc.c
> +++ b/dlls/gdi32/dc.c
> @@ -56,7 +56,7 @@ DC_ATTR *get_dc_attr( HDC hdc )
>  {
>      DWORD type = gdi_handle_type( hdc );
>      DC_ATTR *dc_attr;
> - if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> - if ((type && (type & 0x1f0000) != NTGDI_OBJ_DC) || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> {
>     SetLastError( ERROR_INVALID_HANDLE );
>     return NULL;

This doesn't look right, type mask shouldn't just disappear with no
consequences. I think what has to happen is appropriate wow32 conversion
in default procedure. It has to be applied for every affected message
obviously. Please try attached patch.

[Oleh Nykyforchyn]

I agree that to restore the missing upper part is better. Could You
submit the patch and mark my patch as superceded?

сб, 23 жовт. 2021 о 23:32 Nikolay Sivov  пише:
> On 10/23/21 10:34 AM, Oleh Nykyforchyn wrote:
> > When background is erased in a 16 bit app, a handle to DC is passed
> > through USER16 functions, namely CallWindowProc16 (USER.122) and
> > DefDriverProc16 (USER.255), whose respective parameters are of type
> > WPARAM16, hence the upper part of the handle is cut. Later
> > the returned handle is rejected by get_dc_attr() in GDI32 because
> > its type is 0, although such handles are accepted by handle_entry().
> > This results in black and not cleared background.  Allowing type==0
> > in get_dc_attr() solves the problem.
> >
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51899
> > Signed-off-by: Oleh Nykyforchyn
> >
> > dlls/gdi32/dc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c
> > index 1511c565194..a161b3cb640 100644
> > --- a/dlls/gdi32/dc.c
> > +++ b/dlls/gdi32/dc.c
> > @@ -56,7 +56,7 @@ DC_ATTR *get_dc_attr( HDC hdc )
> >  {
> >      DWORD type = gdi_handle_type( hdc );
> >      DC_ATTR *dc_attr;
> > - if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > - if ((type && (type & 0x1f0000) != NTGDI_OBJ_DC) || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > {
> >     SetLastError( ERROR_INVALID_HANDLE );
> >     return NULL;
>
> This doesn't look right, type mask shouldn't just disappear with no
> consequences. I think what has to happen is appropriate wow32 conversion
> in default procedure. It has to be applied for every affected message
> obviously. Please try attached patch.

[Oleh Nykyforchyn]

I added a TRACE to dc.c:

---------
--- dc.c.prev   2021-10-21 13:47:33.567548593 +0300
+++ dc.c        2021-10-23 08:29:38.003977433 +0300
@@ -56,7 +56,15 @@
 {
     DWORD type = gdi_handle_type( hdc );
     DC_ATTR *dc_attr;
-    if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr =
get_gdi_client_ptr( hdc, 0 )))
+
+    if ((type & 0x1f0000) != NTGDI_OBJ_DC)
+    {
+        TRACE("type %x, cut %x vs %x\n", type, type & 0x1f0000, NTGDI_OBJ_DC);
+        SetLastError( ERROR_INVALID_HANDLE );
+        return NULL;
+    }
+    else
+    if (!(dc_attr = get_gdi_client_ptr( hdc, 0 )))
     {
         SetLastError( ERROR_INVALID_HANDLE );
         return NULL;

-------

and there is still one rejected handle in a log for WinEdt.exe:

0104:trace:win:GetDCEx (00010072,010400B4,0x10088): returning 450100B0 (updated)
0104:trace:gdi:NtGdiCreateSolidBrush 808080
0104:trace:gdi:alloc_gdi_handle allocated NTGDI_OBJ_BRUSH 0xa1000a9 151/65536
0104:trace:gdi:create_brush 0xa1000a9
0104:trace:gdi:SelectObject (000000B0,0A1000A9)
0104:trace:gdi:get_dc_attr type 0, cut 0 vs 10000
0104:trace:gdi:get_dc_attr type 0, cut 0 vs 10000
0104:trace:win:BeginPaint hdc = 450100B0 box = ((0,0)-(1352,594)), fErase = 0
0104:trace:win:release_dc 00010072 450100B0

This one is due to parameters conversion in CallWindowProc16 (USER.122).

Shouldn't this function be patched as well?

нд, 24 жовт. 2021 о 09:50 Oleh Nykyforchyn  пише:
> I agree that to restore the missing upper part is better. Could You
> submit the patch and mark my patch as superceded?
>
> сб, 23 жовт. 2021 о 23:32 Nikolay Sivov  пише:
> > On 10/23/21 10:34 AM, Oleh Nykyforchyn wrote:
> > > When background is erased in a 16 bit app, a handle to DC is passed
> > > through USER16 functions, namely CallWindowProc16 (USER.122) and
> > > DefDriverProc16 (USER.255), whose respective parameters are of type
> > > WPARAM16, hence the upper part of the handle is cut. Later
> > > the returned handle is rejected by get_dc_attr() in GDI32 because
> > > its type is 0, although such handles are accepted by handle_entry().
> > > This results in black and not cleared background.  Allowing type==0
> > > in get_dc_attr() solves the problem.
> > >
> > > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51899
> > > Signed-off-by: Oleh Nykyforchyn
> > >
> > > dlls/gdi32/dc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c
> > > index 1511c565194..a161b3cb640 100644
> > > --- a/dlls/gdi32/dc.c
> > > +++ b/dlls/gdi32/dc.c
> > > @@ -56,7 +56,7 @@ DC_ATTR *get_dc_attr( HDC hdc )
> > >  {
> > >      DWORD type = gdi_handle_type( hdc );
> > >      DC_ATTR *dc_attr;
> > > - if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > > - if ((type && (type & 0x1f0000) != NTGDI_OBJ_DC) || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > > {
> > >     SetLastError( ERROR_INVALID_HANDLE );
> > >     return NULL;
> >
> > This doesn't look right, type mask shouldn't just disappear with no
> > consequences. I think what has to happen is appropriate wow32 conversion
> > in default procedure. It has to be applied for every affected message
> > obviously. Please try attached patch.

[Oleh Nykyforchyn]

If WINPROC_CallProc16To32A is also patched, rejections go away.

нд, 24 жовт. 2021 о 10:19 Oleh Nykyforchyn  пише:
> I added a TRACE to dc.c:
>
> --- dc.c.prev   2021-10-21 13:47:33.567548593 +0300
> +++ dc.c        2021-10-23 08:29:38.003977433 +0300
> @@ -56,7 +56,15 @@
>  {
>      DWORD type = gdi_handle_type( hdc );
>      DC_ATTR *dc_attr;
> - if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr =
>
> get_gdi_client_ptr( hdc, 0 )))
> -
> - if ((type & 0x1f0000) != NTGDI_OBJ_DC)
> - {
> -
>    TRACE("type %x, cut %x vs %x\n", type, type & 0x1f0000, NTGDI_OBJ_DC);
> -
>    SetLastError( ERROR_INVALID_HANDLE );
> -
>    return NULL;
> - }
> - else
> - if (!(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> {
>     SetLastError( ERROR_INVALID_HANDLE );
>     return NULL;
>
> and there is still one rejected handle in a log for WinEdt.exe:
>
> 0104:trace:win:GetDCEx (00010072,010400B4,0x10088): returning 450100B0 (updated)
> 0104:trace:gdi:NtGdiCreateSolidBrush 808080
> 0104:trace:gdi:alloc_gdi_handle allocated NTGDI_OBJ_BRUSH 0xa1000a9 151/65536
> 0104:trace:gdi:create_brush 0xa1000a9
> 0104:trace:gdi:SelectObject (000000B0,0A1000A9)
> 0104:trace:gdi:get_dc_attr type 0, cut 0 vs 10000
> 0104:trace:gdi:get_dc_attr type 0, cut 0 vs 10000
> 0104:trace:win:BeginPaint hdc = 450100B0 box = ((0,0)-(1352,594)), fErase = 0
> 0104:trace:win:release_dc 00010072 450100B0
>
> This one is due to parameters conversion in CallWindowProc16 (USER.122).
>
> Shouldn't this function be patched as well?
>
> нд, 24 жовт. 2021 о 09:50 Oleh Nykyforchyn  пише:
> > I agree that to restore the missing upper part is better. Could You
> > submit the patch and mark my patch as superceded?
> >
> > сб, 23 жовт. 2021 о 23:32 Nikolay Sivov  пише:
> > > On 10/23/21 10:34 AM, Oleh Nykyforchyn wrote:
> > > > When background is erased in a 16 bit app, a handle to DC is passed
> > > > through USER16 functions, namely CallWindowProc16 (USER.122) and
> > > > DefDriverProc16 (USER.255), whose respective parameters are of type
> > > > WPARAM16, hence the upper part of the handle is cut. Later
> > > > the returned handle is rejected by get_dc_attr() in GDI32 because
> > > > its type is 0, although such handles are accepted by handle_entry().
> > > > This results in black and not cleared background.  Allowing type==0
> > > > in get_dc_attr() solves the problem.
> > > >
> > > > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51899
> > > > Signed-off-by: Oleh Nykyforchyn
> > > >
> > > > dlls/gdi32/dc.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c
> > > > index 1511c565194..a161b3cb640 100644
> > > > --- a/dlls/gdi32/dc.c
> > > > +++ b/dlls/gdi32/dc.c
> > > > @@ -56,7 +56,7 @@ DC_ATTR *get_dc_attr( HDC hdc )
> > > >  {
> > > >      DWORD type = gdi_handle_type( hdc );
> > > >      DC_ATTR *dc_attr;
> > > > - if ((type & 0x1f0000) != NTGDI_OBJ_DC || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > > > - if ((type && (type & 0x1f0000) != NTGDI_OBJ_DC) || !(dc_attr = get_gdi_client_ptr( hdc, 0 )))
> > > > {
> > > >     SetLastError( ERROR_INVALID_HANDLE );
> > > >     return NULL;
> > >
> > > This doesn't look right, type mask shouldn't just disappear with no
> > > consequences. I think what has to happen is appropriate wow32 conversion
> > > in default procedure. It has to be applied for every affected message
> > > obviously. Please try attached patch.