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'
asked Mar 4, 2018 at 2:21
\$\endgroup\$
2
  • \$\begingroup\$ is this MASM? as each compiler has it's own things that could be used \$\endgroup\$ Commented Mar 4, 2018 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\$ Commented Mar 4, 2018 at 19:17

1 Answer 1

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'
answered Mar 4, 2018 at 18:01
\$\endgroup\$
1
  • 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\$ Commented Mar 4, 2018 at 20:44

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.