2

My assignment for school is to loop through a sequence of characters in a string and swap them such that the end result is the original string in reverse.

I have written 3 assembly functions and one cpp function but on the function below I am getting a few errors when I try to run the program and I'm not sure how to fix it. I will post both the cpp code and assembly code below with the errors pointed out, if anyone could point out what my mistake is I would appreciate it a lot!

My c++ code is below

#include<iostream>
#include <string>

using namespace std;
extern"C"
char reverse(char*, int);

int main()
{
  char str[64] = {NULL};
  int lenght;

  cout << " Please Enter the text you want to reverse:";
  cin >> str;
  lenght = strlen(str);

  reverse(str, lenght);

  cout << " the reversed of the input is: " << str << endl;

  }

Below is my assembly code

.model flat

.code

_reverse PROC    ;named _test because C automatically prepends an underscode, it is needed to interoperate

     push ebp     
  mov ebp,esp  ;stack pointer to ebp

  mov ebx,[ebp+8]       ; address of first array element
  mov ecx,[ebp+12]  ; the number of elemets in array
  mov eax,ebx   
  mov ebp,0         ;move 0 to base pointer 
  mov edx,0     ; set data register to 0
  mov edi,0

Setup:

  mov esi , ecx
  shr ecx,1
  add ecx,edx
  dec esi

reverse:

  cmp ebp , ecx
  je allDone

  mov edx, eax
  add eax , edi
  add edx , esi

LoopMe:
  mov bl, [edx]
  mov bh, [eax]

  mov [edx],bh
  mov [eax],bl

  inc edi
  dec esi

  cmp edi, esi
  je allDone

  inc ebp
  jmp reverse

allDone:
  pop ebp               ; pop ebp out of stack 
  ret                   ; retunr the value of eax
 _reverse ENDP

END

On the line close to the beginning where it reads push ebp I'm getting an error that says

invalid instruction operands

and towards the end where it reads pop ebp I'm getting an error where it says the same thing.
Not sure if this is big but I'm also getting a syntax error on the very first line of code that reads .model flat.

Sep Roland
  • 33,889
  • 7
  • 43
  • 76
chrisward
  • 23
  • 5
  • I haven't used assemblers for ages. (usually because compilers do such a good job nowadays). It might help if we (others) know which assembler you are using. And probably you need a seperate header file for your asm in the end, and link the object file with your c++ exe. Also be sure to explicitly mention the calling convention (e.g. stdcall) used for your assembly in the function declaration. Note : I think `cmp edi, esi` , `je allDone`, might be tricky when the string has even length. Shouldn't it be `jge` (just when the pointers swap)? – Pepijn Kramer Nov 20 '22 at 10:32
  • @PepijnKramer: if it didn't error on `.code` or `_reverse PROC`, then we can infer this is MASM syntax, presumably assembled with MASM, but possibly JWASM which accepts the same syntax but is open-source and portable. (But yes, the question should have specified, and quoted the exact error messages as part of its [mcve].) Good point about 32-bit Windows using different calling conventions; this appears to be cdecl, as it's just `ret`, not `ret 8`. (And it hasn't used anything that would make MASM magically rewrite your `ret` to `ret 8`.) – Peter Cordes Nov 20 '22 at 11:34
  • 1
    Yes, for the bottom of a `do{front++;back--;}while()` loop you'll need `front < back`, not `front != back` as they can cross. Instead of jumping over a `jmp` like an `if()break` inside an infinite loop, put `cmp esi, edi` / `jb loop_top`. ([Why are loops always compiled into "do...while" style (tail jump)?](https://stackoverflow.com/q/47783926)), and use unsigned for pointer compares. That's the only loop condition you need, the stuff with EBP also counting up to `n>>1` is redundant. I assume it was added to work around the bug where they cross? – Peter Cordes Nov 20 '22 at 11:39
  • 1
    The other bug with this code is that it modifies call-preserved registers without saving/restoring them, so the compiler-generated code in the caller might crash if it had values in ESI, EDI, or EBX. This should be doable with only EAX, ECX, and EDX, if you're willing to use to parts of the same register for two separate bytes like you were doing with EBX. (Two pointers that you walk in towards each other, and two byte registers.) – Peter Cordes Nov 20 '22 at 12:31

1 Answers1

7

Based on reproducing the symptoms, I diagnose the problem as: this is 32-bit x86 assembly (clearly), but it was treated as x64 assembly, and that didn't work.

  • the .model directive is not valid for x64, so there is a syntax error there.
  • pushing and popping 32-bit registers is not encodeable in x64, so there are invalid operand errors there.

If this is in a project in Visual Studio, set the "platform" for either the whole solution or this individual project to x86/win32 (it has different names in different places, but set it to 32-bit).

solution platform selector

harold
  • 61,398
  • 6
  • 86
  • 164
  • That did it! Thank you so much I didn't realize the issue was such a simple fix and I thought it surely had to do with my code – chrisward Nov 20 '22 at 11:50
  • Yeah, I was never a fan of calling it `x64`, I get it and all, but we don't call the 32-bit version `x32`. I feel like it should say `64-bit x86` and `32-bit x86` for additional clarity. – puppydrum64 Nov 23 '22 at 17:13