1

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?

nrz
  • 10,435
  • 4
  • 39
  • 71
gkamani2011
  • 225
  • 1
  • 4
  • 14
  • possible duplicate of [Run-Time Check Failure #0 in embedded asm code](http://stackoverflow.com/questions/5902157/run-time-check-failure-0-in-embedded-asm-code) – Zac Howland Nov 21 '13 at 20:29
  • 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 Nov 21 '13 at 20:32
  • Ya, I just figured out the swapping part. And it works. I just wanted to try and implement stack. – gkamani2011 Nov 21 '13 at 20:40

2 Answers2

3

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.

zwol
  • 135,547
  • 38
  • 252
  • 361
  • I tried doing that, it still gives me the same error. How do you suggest I reverse the string, and it has to be done in assembly. And when I tried debugging it, after ever char it a weird character like 'i' was popped. The character is 'Ì' – gkamani2011 Nov 21 '13 at 20:11
  • I would do it in C using the standard in-place algorithm for reversing strings (see for instance http://stackoverflow.com/a/199891/388520 ). You can translate that to assembly if you really want. I don't know what is going on with the weird characters. – zwol Nov 21 '13 at 20:24
  • 1
    It is actually an assignment where we are required to use inline asm for the reversing part. – gkamani2011 Nov 21 '13 at 20:25
3

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.

greatwolf
  • 20,287
  • 13
  • 71
  • 105
  • Nice! I had just assumed `push al` and `pop al` were valid. Serve me right for not checking the manual. – zwol Nov 21 '13 at 22:06