Access violation on 'ret' instruction

1
votes

I've got this function, which consists mostly of inline asm.

long *toarrayl(int members, ...){
    __asm{
        push esp

        mov eax, members
        imul eax, 4
        push eax
        call malloc
        mov edx, eax
        mov edi, eax

        xor ecx, ecx
        xor esi, esi
loopx:
        cmp ecx, members
        je done
        mov esi, 4

        imul esi, ecx
        add esi, ebp
        mov eax, [esi+0xC]
        mov [edi], eax
        inc ecx
        add edi, 4
        jmp loopx
done:
        mov eax, edx
        pop esp
        ret
    }
}

And upon running, I get an access violation on the return instruction.

I'm using VC++ 6, and it can sometimes mean to point at the line above, so possible on 'pop esp'. If you could help me out, it'd be great. Thanks, iDomo.

3
c++assemblyaccess-violationinline-assembly
Clear the size (eax) off the stack after calling malloc. And leave ret out of your code, let the compiler handle that part. - DCoder
ret crashes? Stack smashing. flies away - user529758
When using inline Assembly, does the compiler automatically save the required registers? If not, you need to save ESI, EDI, EBX, EBP, and ESP at the beginning (if you modify them) with pushes, and restore at the end with pops - Gunner

3 Answers

8
votes

You are failing to manage the stack pointer correctly. In particular, your call to malloc unbalances the stack, and your pop esp ends up popping the wrong value into esp. The access violation therefore occurs when you try to ret from an invalid stack (the CPU cannot read the return address). It's unclear why you are pushing and popping esp; that accomplishes nothing.

0
votes

As you spotted, you should never use the instruction POP ESP - when you see that in the code, you know something extremely wrong has happened. Of course, calling malloc inside asseembler code is also rather a bad thing to do - you have for example forgotten to check if it returned NULL, so you may well crash. Stick that outside your assembler - and check for NULL, it's much easier to debug "Couldn't allocate memory at line 54 in file mycode.c" than "Somewhere in the assembler, we got a

Here's some suggestions for improvement, which should speed up your loop a bit:

long *toarrayl(int members, ...){
    __asm{
        mov eax, members
        imul eax, 4
        push eax
        call malloc
        add esp, 4
        mov edx, eax
        mov edi, eax

        mov ecx, members
        lea esi, [ebp+0xc]
loopx:
        mov eax, [esi]
        mov [edi], eax
        add edi, 4
        add esi, 4
        dec ecx
        jnz loopx
        mov lret, eax
        ret
    }
}

Improvements: Remove multiply by four in every loop. Just increment esi instead. Use decrement on ecx, instead of increament, and load it up with members before the loop. This allows usage of just one jump in the loop, rather than two. Remove reduntant move from edx, to eax. Use eax directly.

0
votes

I've figured out the answer on my own.

For those who have had this same, or alike problem:

The actual exception was occuring after the user code, when vc++ automatically pops/restores the registers to their states before the function was called. Since I miss-aligned the stack pointer when calling malloc, there was an access violation when poping from the stack. I wasn't able to see this in the editor because it wasn't my code, so it was just shown as the last of my code in the function.

To correct this, just add an add esp, (size of parameters for previous call) after the calls you make.

Fixed code:

long *toarrayl(int members, ...){
    __asm{
        mov eax, members
        imul eax, 4
        push eax
        call malloc
        add esp, 4
        mov edx, eax
        mov edi, eax

        xor ecx, ecx
        xor esi, esi
loopx:
        cmp ecx, members
        je done
        mov esi, 4

        imul esi, ecx
        add esi, ebp
        mov eax, [esi+0xC]
        mov [edi], eax
        inc ecx
        add edi, 4
        jmp loopx
done:
        mov eax, edx
        ret
    }
    //return (long*)0;
}

Optimized code:

long *toarrayl(int members, ...){
    __asm{
        mov eax, members
        shl eax, 2
        push eax
        call malloc
        add esp, 4
        ;cmp eax, 0
        ;je _error
        mov edi, eax
        mov ecx, members
        lea esi, [ebp+0xC]
loopx:
        mov edx, [esi]
        mov [edi], edx
        add edi, 4
        add esi, 4
        dec ecx
        jnz loopx
    }
}