[PATCH v9 0/9] 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

--
  v9: improve formatting

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

[PATCH v9 1/9] 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 v9 2/9] 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 v9 3/9] 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 v9 4/9] 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 v9 5/9] 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 v9 6/9] 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 v9 7/9] 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;
         }

[PATCH v9 8/9] check correct number of bytes

[Etaash Mathamsetty]

From: Etaash Mathamsetty

might be an issue?
---
 dlls/ntoskrnl.exe/instr.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index 2205def6be5..0122672e9fb 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -893,34 +893,42 @@ 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);
-        SIZE_T offset = data - user_shared_data;
-        SIZE_T offset2 = data2 - user_shared_data;
+        ULONG64* data = (ULONG64*)INSTR_GetOperandAddr(context, instr + 1, prefixlen + 1, long_addr, rex, segprefix, &len);
+        ULONG64* data2 = (ULONG64*)INSTR_GetOperandAddr(context, instr + 2, prefixlen + 2, long_addr, rex, segprefix, &len);
+        SIZE_T offset = (BYTE*)data - user_shared_data;
+        SIZE_T offset2 = (BYTE*)data2 - user_shared_data;
         SIZE_T data_size = get_op_size(long_op, rex);
         BOOL is_user_shared_data = FALSE;
+        ULONG64 cmp1, cmp2, bitmask;
         if(offset <= KSHARED_USER_DATA_PAGE_SIZE - data_size)
         {
             TRACE("USD offset %#x at %p\n",(unsigned int) offset, (void*) context->Rip);
-            data = (BYTE*)(wine_user_shared_data + offset);
+            data = (ULONG64*)(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\n", (unsigned int) offset2, (void*) context->Rip);
-            data2 = (BYTE*)(wine_user_shared_data + offset2);
+            data2 = (ULONG64*)(wine_user_shared_data + offset2);
             is_user_shared_data = TRUE;
         }
-        if(is_user_shared_data){
+        if(is_user_shared_data)
+        {
             /* clear ZF and CF */
             context->EFlags &= ~(1UL << 6);
             context->EFlags &= ~(1UL);

-            if(*data == *data2)
+            bitmask = ((1ULL << (8 * data_size)) - 1); 
+            /* FIXME("bitmask %llx\n", bitmask); */
+
+            cmp1 = (*data) & bitmask; 
+            cmp2 = (*data2) & bitmask;
+
+            if(cmp1 == cmp2)
             {
                 context->EFlags |= (1UL << 6); /* ZF */
             }
-            else if(*data < *data2){
+            else if(cmp1 < cmp2){
                 context->EFlags |= 1UL; /* CF */
             }
             context->Rip += prefixlen + len + 1;

[PATCH v9 9/9] improve formatting

[Etaash Mathamsetty]

From: Etaash Mathamsetty

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

diff --git a/dlls/ntoskrnl.exe/instr.c b/dlls/ntoskrnl.exe/instr.c
index 0122672e9fb..33ade6799ab 100644
--- a/dlls/ntoskrnl.exe/instr.c
+++ b/dlls/ntoskrnl.exe/instr.c
@@ -925,12 +925,10 @@ static DWORD emulate_instruction( EXCEPTION_RECORD *rec, CONTEXT *context )
             cmp2 = (*data2) & bitmask;

if(cmp1 == cmp2)
-            {
                 context->EFlags |= (1UL << 6); /* ZF */
-            }
-            else if(cmp1 < cmp2){
+            else if(cmp1 < cmp2)
                 context->EFlags |= 1UL; /* CF */
-            }
+
             context->Rip += prefixlen + len + 1;
             return ExceptionContinueExecution;
         }

[Bernhard Kölbl (＠besentv)]

On Wed Aug 24 23:23:42 2022 +0000, Etaash Mathamsetty wrote:
> I fixed all the warnings as well...

The build task still shows warnings.

[Robert Wilhelm (＠sloper42)]

On Thu Aug 25 07:07:14 2022 +0000, Bernhard K��lbl wrote:
> The build task still shows warnings.

Hi Etaash, you have to squash the formatting and whitespace changes into the first commit. The test build will run after each commit and bail out if there are errors and warnings.