[PATCH v9 0/9] MR706: Draft: ntoskrnl: implement cmp instruction with opcodes 0x38,0x39
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
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> 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 */ { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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){ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
From: Etaash Mathamsetty <etaash.mathamsetty(a)gmail.com> --- 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/706
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/706#note_6885
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/706#note_6889
participants (4)
-
Bernhard Kölbl -
Etaash Mathamsetty
-
Etaash Mathamsetty (＠etaash.mathamsetty)
-
Robert Wilhelm (＠sloper42)