[PATCH v7 0/7] MR706: Draft: ntoskrnl: implement cmp instruction with opcodes 0x38,0x39

[Etaash Mathamsetty (＠etaash.mathamsetty)]

fixes bug #48988
I am not very familiar with assembly, so feedback is appreciated

--
  v7: fix missing newline

https://gitlab.winehq.org/wine/wine/-/merge_requests/706

[PATCH v7 1/7] implement cmp instruction

[Etaash Mathamsetty]

From: Etaash Mathamsetty

with opcode 0x38 and 0x39
---
 dlls/ntoskrnl.exe/instr.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index 8f1aa4d45a3..c9c61513a33 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -890,7 +890,39 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
         }
         break;  /* Unable to emulate it */
     }
+    case 0x38:
+    case 0x39: /* cmp r/m r/m*/
+    {
+        BYTE *data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
+        BYTE *data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
+        SIZE_T offset = data - user_shared_data;
+        SIZE_T offset2 = data2 - user_shared_data;
+        SIZE_T data_size = get_op_size(long_op, rex);
+        if(offset <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
+        {
+            if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
+            {
+                data2 = wine_user_shared_data + offset2;
+                TRACE("USD offset2 %#x at %#p", (unsigned int)offset, (void*)context->Rip);
+            }
+            TRACE("USD offset %#x at %#p\n",(unsigned int) offset, (void*)context->Rip);
+
+            /* clear ZF and CF */
+            context->EFlags &= ~(1UL << 6);
+            context->EFlags &= ~(1UL);

+            if(*(wine_user_shared_data + offset) == *data2)
+            {
+                context->EFlags |= (1UL << 6); /* ZF */
+            }
+            else if(*(wine_user_shared_data + offset) < *data2){
+                context->EFlags |= 1UL; /* CF */
+            }
+            context->Rip += prefixlen + len + 1;
+            return ExceptionContinueExecution;
+        }
+        break;
+    }
     case 0xa0: /* mov Ob, AL */
     case 0xa1: /* mov Ovqp, rAX */
     {

[PATCH v7 2/7] maybe fixes error?

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index c9c61513a33..dfb79830b7a 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -893,8 +893,10 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
     case 0x38:
     case 0x39: /* cmp r/m r/m*/
     {
-        BYTE *data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
-        BYTE *data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
+        BYTE* data = NULL;
+        BYTE* data2 = NULL;
+        data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
+        data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
         SIZE_T offset = data - user_shared_data;
         SIZE_T offset2 = data2 - user_shared_data;
         SIZE_T data_size = get_op_size(long_op, rex);

[PATCH v7 3/7] fix print formatting

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index dfb79830b7a..b4c646bb3b2 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -905,9 +905,9 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
             if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
             {
                 data2 = wine_user_shared_data + offset2;
-                TRACE("USD offset2 %#x at %#p", (unsigned int)offset, (void*)context->Rip);
+                TRACE("USD offset2 %#x at %p", (unsigned int)offset, (void*)context->Rip);
             }
-            TRACE("USD offset %#x at %#p\n",(unsigned int) offset, (void*)context->Rip);
+            TRACE("USD offset %#x at %p\n",(unsigned int) offset, (void*)context->Rip);

/* clear ZF and CF */
             context->EFlags &= ~(1UL << 6);

[PATCH v7 4/7] fix potential logic error

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index b4c646bb3b2..55a3ea8d0d3 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -893,31 +893,34 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
     case 0x38:
     case 0x39: /* cmp r/m r/m*/
     {
-        BYTE* data = NULL;
-        BYTE* data2 = NULL;
-        data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
-        data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
+        BYTE* data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
+        BYTE* data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
         SIZE_T offset = data - user_shared_data;
         SIZE_T offset2 = data2 - user_shared_data;
         SIZE_T data_size = get_op_size(long_op, rex);
+        BOOL is_user_shared_data = FALSE;
         if(offset <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
         {
-            if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
-            {
-                data2 = wine_user_shared_data + offset2;
-                TRACE("USD offset2 %#x at %p", (unsigned int)offset, (void*)context->Rip);
-            }
-            TRACE("USD offset %#x at %p\n",(unsigned int) offset, (void*)context->Rip);
-
-            /* clear ZF and CF */
+            TRACE("USD offset %#x at %p\n",(unsigned int) offset, (void*) context->Rip);
+            data = wine_user_shared_data + offset;
+            is_user_shared_data = TRUE;
+        }
+        if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
+        {
+            TRACE("USD offset %#x at %p", (unsigned int) offset2, (void*) context->Rip);
+            data2 = wine_user_shared_data + offset2;
+            is_user_shared_data = TRUE;
+        }
+        if(is_user_shared_data){
+             /* clear ZF and CF */
             context->EFlags &= ~(1UL << 6);
             context->EFlags &= ~(1UL);

-            if(*(wine_user_shared_data + offset) == *data2)
+            if(*data == *data2)
             {
                 context->EFlags |= (1UL << 6); /* ZF */
             }
-            else if(*(wine_user_shared_data + offset) < *data2){
+            else if(*data < *data2){
                 context->EFlags |= 1UL; /* CF */
             }
             context->Rip += prefixlen + len + 1;

[PATCH v7 5/7] fix errors

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index 55a3ea8d0d3..36a1f9c93aa 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -902,13 +902,13 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
         if(offset <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
         {
             TRACE("USD offset %#x at %p\n",(unsigned int) offset, (void*) context->Rip);
-            data = wine_user_shared_data + offset;
+            data = (BYTE*)(wine_user_shared_data + offset);
             is_user_shared_data = TRUE;
         }
         if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
         {
             TRACE("USD offset %#x at %p", (unsigned int) offset2, (void*) context->Rip);
-            data2 = wine_user_shared_data + offset2;
+            data2 = (BYTE*)(wine_user_shared_data + offset2);
             is_user_shared_data = TRUE;
         }
         if(is_user_shared_data){

[PATCH v7 6/7] remove extra space

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index 36a1f9c93aa..b7682fe9afc 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -912,7 +912,7 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
             is_user_shared_data = TRUE;
         }
         if(is_user_shared_data){
-             /* clear ZF and CF */
+            /* clear ZF and CF */
             context->EFlags &= ~(1UL << 6);
             context->EFlags &= ~(1UL);

[PATCH v7 7/7] fix missing newline

[Etaash Mathamsetty]

From: Etaash Mathamsetty

---
 dlls/ntoskrnl.exe/instr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index b7682fe9afc..2205def6be5 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -891,7 +891,7 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
         break;  /* Unable to emulate it */
     }
     case 0x38:
-    case 0x39: /* cmp r/m r/m*/
+    case 0x39: /* cmp r/m r/m */
     {
         BYTE* data = INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
         BYTE* data2 = INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
@@ -907,7 +907,7 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
         }
         if(offset2 <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
         {
-            TRACE("USD offset %#x at %p", (unsigned int) offset2, (void*) context->Rip);
+            TRACE("USD offset %#x at %p\n", (unsigned int) offset2, (void*) context->Rip);
             data2 = (BYTE*)(wine_user_shared_data + offset2);
             is_user_shared_data = TRUE;
         }

[Bernhard Kölbl (＠besentv)]

On Wed Aug 24 21:32:27 2022 +0000, Etaash Mathamsetty wrote:
> why is the test build failing, I fixed all the errors and the diff is up
> to date?!?

```
cc1: all warnings being treated as errors
make: *** [Makefile:105308: dlls/ntoskrnl.exe/instr.cross.o] Error 1
make: *** Waiting for unfinished jobs....
```

[Etaash Mathamsetty (＠etaash.mathamsetty)]

On Wed Aug 24 21:32:27 2022 +0000, Bernhard K��lbl wrote:
> cc1: all warnings being treated as errors
> make: *** [Makefile:105308: dlls/ntoskrnl.exe/instr.cross.o] Error 1
> make: *** Waiting for unfinished jobs....

I fixed all the warnings as well...