C++ inline assembly run time check failure #0

1
votes

I am working on a c++ code that needs to have inline assembly code to reverse the string. So if my input is : "qwerasd" the output should be "dsarewq". I thought of implementing this using stacks. My code is:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void reverseString(char* buffer, int len){

__asm {

    push ecx
    push edi
    push eax

    mov     ecx, len
    mov     edi, buffer

 top:
    mov     al, BYTE PTR [edi]
    push    al
    inc     edi
    loop    top

    mov     ecx, len
    mov     edi, buffer

refill:
    pop     al
    mov     BYTE PTR [edi], al
    inc     edi
    loop    refill
  }
}

int main()  {

    char    s[64];
    char    *ptr = s;
    int size;

    printf("\nEnter text: ");
    scanf("%s", s);
    size = strlen(s);

    reverseString(ptr, size);

    printf("\nThe new text is: %s\n\n", s);
    exit(0);
}

I am trying to push the character one by one onto the stack and then just popping them one by one and storing it back in the string.

When I run the code I get the following error: Run-Time Check Failure #0 - The value of ESP was not properly saved across a function call. This is usually a result of calling a function declared with one calling convention with a function pointer declared with a different calling convention.

What am I doing wrong?

2
c++assemblyx86inline-assembly
possible duplicate of Run-Time Check Failure #0 in embedded asm code - Zac Howland
I'm assuming this is an academic assignment (as there would be no sane reason to do this in inline assembly), so with that in mind, there is also no reason to use a stack. You can do it in a single loop and swapping characters. - Zac Howland
Ya, I just figured out the swapping part. And it works. I just wanted to try and implement stack. - gkamani2011

2 Answers

3
votes

It seems mildly ludicrous to me that you would want to do this in assembly language, but whatever. Your problem is right here:

push ecx
push edi
push eax

You never popped those off the stack! Everything you put on the stack, you must take back off again before you leave the __asm { ... } block.

Note that it may not be necessary to save all of those registers. I'm not 100% sure about this, but I am under the impression that the Windows ABI says eax, ecx, and edx are call clobbered, meaning you don't have to save and restore them in a leaf procedure like this one.

3
votes

In addition to Zack's answer, pushing ediactually isn't necessary because it seems the compiler automatically does that for you, at least for msvc.

The other issue with your code above is that you're doing a pop al which will only increment esp by one byte. This obviously causes a problem because normal push pop operations are expected to be 4-byte aligned under 32-bits. When your function exits, it ends up restoring the wrong value for ebp, it returns to some erroneous return address and havoc ensues.

You can fix it this way:

top:
  movzx   eax, BYTE PTR [edi]
  push    eax
// ...
refill:
  pop     eax
  mov     BYTE PTR [edi], al

Edit: Just to add some clarification, al is not a valid operand size to use on push pop. Not sure why msvc didn't give an error on this. Your above assembly ends up getting translated into push eax and pop ax instead which is where the imbalance comes from.