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'
-
\$\begingroup\$ is this MASM? as each compiler has it's own things that could be used \$\endgroup\$Paweł Łukasik– Paweł Łukasik2018年03月04日 16:57:38 +00:00Commented 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\$the_endian– the_endian2018年03月04日 19:17:10 +00:00Commented Mar 4, 2018 at 19:17
1 Answer 1
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'
-
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– Edward2018年03月04日 20:44:39 +00:00Commented Mar 4, 2018 at 20:44