Proper way move register values in assembly from one location to another

Question

I'm currently having some unexplained register issues which I can't figure my guess is I'm moving registers improperly from one register to another one.

I'm trying to get the value of EDX into a test.myEDX which mostly always puts the wrong value of EDX into test.myEDX, but part of that EDX value seems right which is very strange some kind of Hi/Lo Bits issues I assume.

Let me explain what I'm trying to do.
First I hook a location in memory which contains some arbitrary assembly code.

I hook it by placing a hardware breakpoint at that location then I can see all the register values at that location in time.
Now I made a simple program that everytime the cpu goes into that breakpointed location I put the EDX value into a struct in C++.
As far as i am aware nothing should modify the EDX register while I am putting it into the struct unless putting values into the struct modifies the EDX which I tested isn't the case, I tested by moving the EDX first into EAX then passing EAX into struct which should have the same value as EDX and it's still wrong maybe EAX is used as well when putting data into the struct? I didn't go further then that in testing all registers to find which one isn't used, doubt that's even the problem anyways.

Another thing to take into consideration to actually do any operations like putting EDX into the struct I have to make in C++ the current EIP go into my naked function, I know from previously doing this stuff that naked functions don't modify any registers at all, they are simply used as a way to extend the current asm code at that EIP into much larger asm code without any trash that C++ would add when going into subroutines.

I also use PUSHAD and PUSHFD to restore previously set register values when I finish dumping EDX into struct it's all in a safe environment (as long as I don't use PUSH/POP's incorrectly of course) before I call POPFD then POPAD

I barely know any ASM but from looking at many examples I never see registers moved like this. (which obviously wouldn't make any sense to duplicate a register, but even the same register moved into 2 different addresses one after the one I haven't seen that).

MOV EBX, EDX
MOV ECX, EDX

But in fact I see something like this (which I thought was my problem why it wasn't working), (it wasn't, I am still clueless what I am doing wrong)

MOV EBX, EDX 
MOV EAX, EDX //Theory: in order to move EDX a second time into ECX, I must not move EDX directly into ECX.
MOV ECX, EAX

I don't see any difference except now EAX is lost, but I still figured I have to re-route every multiple moving of the same register to multiple locations by moving it over and over before actually moving to the original location, silly but that's what I thought you had to do I still think this is the proper way to this job.
Either way this still hasn't helped me I still get wrong values.

I think either those option's AL BL registers screw up EDX or maybe TEST or JE messes it up I can't really tell, I pasted this code into OllyDBG and seems like nothing modified EDX mystery why EDX is wrong, Maybe the address updates it's value too slow? since it's based on RAM speed which doesn't sync up with CPU speed (all stupid theories of course).

Anyways that's about everything I can explain here is the code.

struct TestStruct {
  int myEDX;
  int mySetEDX;
} test;

extern bool optionOne = false;
extern bool optionTwo = false;
DWORD gotoDumpBackAddress = 0x40012345;

void __declspec( naked ) dump(void) {
    __asm {
        PUSHAD
        PUSHFD
        XOR EAX, EAX //zero EAX, used below as AL (optionOne)
        XOR EBX, EBX //zero EBX, used below as BL (optionTwo)
        MOV AL, optionOne //optionOne set
        MOV BL, optionTwo //optionTwo set

        TEST EAX, EAX //Tests if optionOne equals == 0, then je will be equal.
        je gotoOptionOne //Jumps if optionOne equals 0.
        TEST EBX, EBX //Tests if optionTwo equals == 0, then je will be equal.
        je gotoOptionTwo  //Jumps if optionTwo equals 0.

gotoOptionOne:
//This the default case (improper value in EDX..., I could just use address at [ESI+0x2] which is old EDX, which is risky since it's delayed (outdated)
            MOV DWORD PTR DS:[ESI+0x2], EDX //(normal operation)
            MOV test.myEDX, EDX //Stores freshest EDX to test.myEDX (wrong EDX value)
            JMP finish  //Clear temporary used registers and go back to next asm code

//Special case. (works mostly properly)
//Thing is EDX gets updated very frequently, Causes no side-effect only because
//[ESI+0x2] gets updated in a different location as well a bit later to renew the value.
//So it's not even noticeable, but when I run this at it's peak speeds, you start to see the flickering effect, which isn't normal, if I run peak speeds without hook.
//I eliminated the problem that the hook could cause the flicker effect since after
//I call a empty naked Hook with just return address to same location and disable hook
//Then re-enable hook and repeat step above (no flicker occurs).
gotoOptionTwo:
            //MOV DWORD PTR DS:[ESI+0x2], EDX //(normal operation), omitted
            MOV EAX, DWORD PTR DS:[ESI+0x2] //Old EDX, but atleast it's correct.
            MOV test.myEDX, EAX //Stores old EDX into struct test.myEDX
            MOV EAX, test.mySetEDX //Replace old EDX with what I wish it should be.
            MOV DWORD PTR DS:[ESI+0x2], EAX //nySetEDX into what EDX should of did.
            JMP finish //Clear temporary used registers and go back to next asm code
finish:
        POPFD
        POPAD

        JMP gotoDumpBackAddress //return to starting location before dump + 1.

    }
}

EDIT: Okay I haven't explained how I test this code out.

I'm not going to explain how I do the Hardware breakpoint, I don't want this method to be public on the internet, for my own security reasons in the future.
But it works by calling another DLL which communicates with a system driver.

But this should explain how I test it.

I run in a different thread in this DLL Injection.

void diffThread() {
  while(1) {
    if(GetAsyncKeyState(VK_NUMPAD0)) {
      optionOne != optionOne;
      Sleep(1000);
    }

    if(GetAsyncKeyState(VK_NUMPAD1)) {
      optionTwo != optionTwo;
      Sleep(1000);
    }

    Sleep(10);
  }
}

bool __stdcall DllMain(HINSTANCE hInst,DWORD uReason,void* lpReserved)
{
    if(uReason == DLL_PROCESS_ATTACH)
    {
        CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)&diffThread,0 ,NULL,NULL); 
    }
    else if(uReason == DLL_PROCESS_DETACH) 
    { 
        ExitThread(0);
    } 
    return true;
}

Answer 1

Okay honestly this is wrong on so many levels, I mean the code itself. What you are trying to do is to force your way into C territory telling the compiler to back off and you do your own thing by handwriting the code. This may come handy sometimes but you have to keep in mind that you also loose some C features especially things that are done for you automatically and as far as I know the Microsoft compile just doesnt like you messing around in inline assmebly ( don't get me wrong the MASM compiler is great but it just doesn't play along well with the C compiler )

// doing this is not the best since the compiler may
// do some nasty stuff like aligning the struct that
// you have to take watch out for in asm
struct TestStruct {
      int myEDX;
      int mySetEDX;
    } test;

extern bool optionOne = false;
extern bool optionTwo = false;
DWORD gotoDumpBackAddress = 0x40012345; // static memory address? this should fail like instantly, I dont really know what this is supposed to be

void __declspec( naked ) dump(void) {
    __asm {
        PUSHAD // you shouldn't normally push all registers only the ones you actually used!
        PUSHFD
        XOR EAX, EAX // this is how you zero out but honestly you can use EAX's low and high parts as 2 16bit regsiters instead of using 2 32bits for 2 bits of data
        XOR EBX, EBX
        MOV AL, optionOne //optionOne set
        MOV BL, optionTwo //optionTwo set

        TEST EAX, EAX // This check is meaning less because if gotoOptionTwo is false you move on regardless of gotoOpeionOne's value
        je gotoOptionOne
        TEST EBX, EBX // This test should always be true isn't it?
        je gotoOptionTwo

gotoOptionOne:
// Assuming that ESI is coming from the system as you said that there is a
// breakpoint that invokes this code, do you even have access to that part of the memory?
            MOV DWORD PTR DS:[ESI+0x2], EDX
            MOV test.myEDX, EDX // this is just a C idom 'struct' this doesnt really exist in ASM
            JMP finish

gotoOptionTwo:
            MOV EAX, DWORD PTR DS:[ESI+0x2]
            MOV test.myEDX, EAX
            MOV EAX, test.mySetEDX
            MOV DWORD PTR DS:[ESI+0x2], EAX
            JMP finish
finish:
        POPFD
        POPAD

        JMP gotoDumpBackAddress //return to starting location before dump + 1.

    }
}

So to answer your question,

I think the biggest problem here is this line

MOV test.myEDX, EDX

Generally in ASM there is no such thing as a scope, like you're telling it to get you a value of myEDX from test which means get the pointer to the value test and then get the value by the pointer. It may work but it's up to the C compiler to help you out.

So normally you would need to get a pointer and use that to move the data like

LEA ecx,test // get the address of test
MOV DWORD PTR[ecx],edx // use that address to move the data into the struct

Now you are telling it to put that data into the struct, however this makes some assumptions, like here we know that the first element is the one we want to put the data into ( myEDX ) and we assume that a DWORD is the same as an int in C ( on Windows it usually is ) but again assumptions are bad!

Answer 2

Some things that I noticed in your code:

First of all, I don't think that you really need to do the pushad and pushfd You are only using one register really, because ebx wouldn't even be needed, and eax is not considered by the compiler to be preserved anyway.

    XOR EAX, EAX //zero EAX, used below as AL (optionOne)
    MOV AL, optionOne //optionOne set
    TEST EAX, EAX //Tests if optionOne equals == 0, then je will be equal.
    je gotoOptionOne //Jumps if optionOne equals 0.

    MOV AL, optionTwo //optionTwo set
    TEST EAX, EAX //Tests if optionTwo equals == 0, then je will be equal.
    je gotoOptionTwo  //Jumps if optionTwo equals 0.

Your assumption

 MOV EBX, EDX 
 MOV EAX, EDX //Theory: in order to move EDX a second time into ECX, I must not move EDX directly into ECX.
 MOV ECX, EAX

is simply wrong. You can move the same register as often as you like, there is no limitation. The only thing that MIGHT be of consideration is when you are looking for maximum performance because of the instruction scheduling, but this is an entirely different issue and doesn't apply here anyway.

So simply doing

 MOV EBX, EDX 
 MOV ECX, EDX 

would be fine.

You are using ESI but it is nowhere initialized. Maybe this is correct, as I don't really understand what your code is supposed to do.

The you are using the variable test here

MOV test.myEDX, EAX
MOV EAX, test.mySetEDX

but this variable isn't even declared, so where should the compiler know it's address from? And for using it as an offset then you would have to use a register as the base address.