[PATCH] krnl386.exe16: Check for pending DPMI events in call16_handler().

[Zebediah Figura]

If a #GP fault is generated in 16-bit code while (V)IF is clear, we insert a
pending event check. However, a #GP can be generated by simply accessing
invalid memory, and if this occurs winedbg will report the address of
DPMI_PendingEventCheck() rather than the address of the #GP fault.

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/krnl386.exe16/wowthunk.c |  2 +-
 tools/winebuild/relay.c       | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/dlls/krnl386.exe16/wowthunk.c b/dlls/krnl386.exe16/wowthunk.c
index 539bc74..9a54170 100644
--- a/dlls/krnl386.exe16/wowthunk.c
+++ b/dlls/krnl386.exe16/wowthunk.c
@@ -243,7 +243,7 @@ static DWORD call16_handler( EXCEPTION_RECORD *record, EXCEPTION_REGISTRATION_RE
              * emulated because the instruction emulation requires
              * original CS:IP and the emulation may change TEB.dpmi_vif.
              */
-            if(get_vm86_teb_info()->dpmi_vif)
+            if (get_vm86_teb_info()->dpmi_vif && get_vm86_teb_info()->vm86_pending)
                 insert_event_check( context );
 
             if (ret != ExceptionContinueSearch) return ret;
diff --git a/tools/winebuild/relay.c b/tools/winebuild/relay.c
index 4b78f68..94fd756 100644
--- a/tools/winebuild/relay.c
+++ b/tools/winebuild/relay.c
@@ -36,9 +36,6 @@
 /* fix this if the x86_thread_data structure is changed */
 #define GS_OFFSET  0x1d8  /* FIELD_OFFSET(TEB,SystemReserved2) + FIELD_OFFSET(struct x86_thread_data,gs) */
 
-#define DPMI_VIF_OFFSET      (0x1fc + 0) /* FIELD_OFFSET(TEB,GdiTebBatch) + FIELD_OFFSET(WINE_VM86_TEB_INFO,dpmi_vif) */
-#define VM86_PENDING_OFFSET  (0x1fc + 4) /* FIELD_OFFSET(TEB,GdiTebBatch) + FIELD_OFFSET(WINE_VM86_TEB_INFO,vm86_pending) */
-
 static void function_header( const char *name )
 {
     output( "\n\t.align %d\n", get_alignment(4) );
@@ -770,13 +767,6 @@ static void BuildPendingEventCheck(void)
 
     function_header( "DPMI_PendingEventCheck" );
 
-    /* Check for pending events. */
-
-    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", VM86_PENDING_OFFSET );
-    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
-    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", DPMI_VIF_OFFSET );
-    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
-
     /* Process pending events. */
 
     output( "\tsti\n" );
-- 
2.7.4

[Alexandre Julliard]

Zebediah Figura <z.figura12(a)gmail.com> writes:
> -    /* Check for pending events. */
> -
> -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", VM86_PENDING_OFFSET );
> -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
> -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", DPMI_VIF_OFFSET );
> -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );

This doesn't seem right. Are you sure it's safe to remove these checks?

-- 
Alexandre Julliard
julliard(a)winehq.org

[Zebediah Figura]

On 12/04/2017 05:44 AM, Alexandre Julliard wrote:
> Zebediah Figura <z.figura12(a)gmail.com> writes:
> > -    /* Check for pending events. */
> > -
> > -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", VM86_PENDING_OFFSET );
> > -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
> > -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", DPMI_VIF_OFFSET );
> > -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
>
> This doesn't seem right. Are you sure it's safe to remove these checks?

As far as I can tell, those flags shouldn't be modified from 32-bit
code. But I could be wrong.

Do you know of a program that relies on pending interrupts?

[Alexandre Julliard]

Zebediah Figura <z.figura12(a)gmail.com> writes:
> On 12/04/2017 05:44 AM, Alexandre Julliard wrote:
> > Zebediah Figura <z.figura12(a)gmail.com> writes:
> > > -    /* Check for pending events. */
> > > -
> > > -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", VM86_PENDING_OFFSET );
> > > -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
> > > -    output( "\t.byte 0x64\n\ttestl $0xffffffff,(%d)\n", DPMI_VIF_OFFSET );
> > > -    output( "\tje %s\n", asm_name("DPMI_PendingEventCheck_Cleanup") );
> >
> > This doesn't seem right. Are you sure it's safe to remove these checks?
>
> As far as I can tell, those flags shouldn't be modified from 32-bit
> code. But I could be wrong.
>
> Do you know of a program that relies on pending interrupts?

I'm not sure how 32-bit code is relevant, this is used by DOS apps that
use DPMI and handle hardware events. Since we add an event check when
handling a VM86_STI exception, it wouldn't surprise me if removing the
checks above caused an infinite loop.

-- 
Alexandre Julliard
julliard(a)winehq.org