5
\$\begingroup\$

This is my first "real" x86 program developed from scratch. Please critique:

format PE console
entry start

include 'win32a.inc'

section '.text' code executable
start:  pushf
        push hello
        call [printf]
        pop ecx
        pop eax
check_carry_flag:
        mov dx, ax ; ax has flags reg
        test ax, 1h 
        jnz carry_flag_set
        mov esi, edx
check_parity_flag:
        mov eax, esi
        test ax, 4h
        jnz parity_flag_set
        mov esi, edx
check_aux_flag:
        mov eax, esi
        test ax, 10h
        jnz aux_flag_set
        mov esi, edx
check_zero_flag:
        mov eax, esi
        test ax, 40h
        jnz zero_flag_set
        mov esi, edx
check_sign_flag:
        mov eax, esi
        test ax, 80h
        jnz sign_flag_set
        mov esi, edx
check_dir_flag:
        mov eax, esi
        test ax, 400h
        jnz dir_flag_set
        mov esi, edx
check_OF_flag:
        mov eax, esi
        test ax, 800h
        jnz OF_flag_set
jmp endme

carry_flag_set:
        mov esi, edx
        push carry_flag
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_parity_flag
parity_flag_set:
        mov esi, edx
        push parity_flag
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_aux_flag
aux_flag_set:
        mov esi, edx
        push aux_flag
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_zero_flag
zero_flag_set:
        mov esi, edx
        push zero_flag
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_sign_flag
sign_flag_set:
        mov esi, edx
        push sign_flag
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_dir_flag
dir_flag_set:
        mov esi, edx
        push dir_flag_set
        call [printf]
        pop ecx
        mov edx, esi
        jmp check_OF_flag
OF_flag_set:
        mov esi, edx
        push OF_flag
        call [printf]
        pop ecx
endme:
        push p
        call [system]
        pop ecx

        push 0
        call [ExitProcess]
section '.rdata' data readable
        hello db 'The flags are: %X', 10, 0
        p db 'pause>nul'
        carry_flag db 'Carry flag set', 0
        parity_flag db 'Parity flag set', 0
        zero_flag db 'Zero flag set', 0
        sign_flag db 'Sign flag set', 0
        dir_flag db 'Direction flag set', 0
        OF_flag db 'Overflow flag set', 0
        aux_flag db 'Aux flag set', 0

section '.idata' data readable import
        library kernel32, 'kernel32.dll', \
                msvcrt,   'msvcrt.dll'
        import kernel32, ExitProcess, 'ExitProcess'
        import msvcrt, printf, 'printf', \
        system, 'system'
\$\endgroup\$
  • \$\begingroup\$ is this MASM? as each compiler has it's own things that could be used \$\endgroup\$ – Paweł Łukasik Mar 4 '18 at 16:57
  • \$\begingroup\$ @PawełŁukasik sorry I tried to tag as fasm but the forum wouldn't let me, it said I was making a new tag. \$\endgroup\$ – the_endian Mar 4 '18 at 19:17
3
\$\begingroup\$

You have a bit of code duplication that could and should be removed to avoid mistakes. FASM supports macros that you can use to remove duplicated parts.

One can define a check_flag macro:

macro check_flag op1, op2
{
   local flag_not_set
   test eax, op1
   jz flag_not_set
   push eax
   push op2
   call [printf]
   pop eax
   pop eax 
flag_not_set:
}

A bit of explanation. This macro will do all the work that is required to check if a specific flag is set (specified as operand op1) and if so print the message specified by operand op2. We need to specify also our label as local so that there is no label duplication when macro is placed in code multiple times. We also keep the flags to be restored after printing.

Having this we can simplify the code a bit to basically this:

    check_flag 1h, carry_flag
    check_flag 4h, parity_flag
    check_flag 10h, aux_flag
    check_flag 40h, zero_flag
    check_flag 80h, sign_flag
    check_flag 400h, dir_flag
    check_flag 800h, OF_flag    

Which such approach it is easy to add new ones, just by writing:

    check_flag 200h, interrupt_flag

You could also add a new line to those strings so that, they are a bit separated from each other when printing like this:

  aux_flag db 'Aux flag set', 13, 10, 0 

The whole program:

format PE console
entry start

include 'win32a.inc'

section '.text' code executable

macro check_flag op1, op2
{
   local flag_not_set
   test eax, op1
   jz flag_not_set
   push eax
   push op2
   call [printf]
   pop eax
   pop eax
flag_not_set:
}

start:  pushf
    push hello
    call [printf]
    pop ecx
    pop eax

    check_flag 1h, carry_flag
    check_flag 4h, parity_flag
    check_flag 10h, aux_flag
    check_flag 40h, zero_flag
    check_flag 80h, sign_flag
    check_flag 400h, dir_flag
    check_flag 800h, OF_flag

    push p
    call [system]
    pop ecx

    push 0
    call [ExitProcess]

section '.rdata' data readable
    hello db 'The flags are: %X', 10, 0
    p db 'pause>nul'
    carry_flag db 'Carry flag set', 13,10,0
    parity_flag db 'Parity flag set',13,10, 0
    zero_flag db 'Zero flag set',13,10, 0
    sign_flag db 'Sign flag set',13,10, 0
    dir_flag db 'Direction flag set',13,10, 0
    OF_flag db 'Overflow flag set',13,10, 0
    aux_flag db 'Aux flag set',13,10, 0

section '.idata' data readable import
    library kernel32, 'kernel32.dll', \
            msvcrt,   'msvcrt.dll'
    import kernel32, ExitProcess, 'ExitProcess'
    import msvcrt, printf, 'printf', \
    system, 'system'
\$\endgroup\$
  • 1
    \$\begingroup\$ Good answer. I’m even lazier and would have had the flag mask as part of the data structure, with a mask value of zero signifying end-of-table. Then it becomes a simple data-driven loop which keeps related items together. \$\endgroup\$ – Edward Mar 4 '18 at 20:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.