Strange Warning

Index > Main > Strange Warning

Author

Thread

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 07 Jan 2010, 03:18

Assume 16bit code

Command input buffer

Code:

g_CmdBuf db 8+1+3+1 dup(0) ; filename,dot,ext,NULL

Command string table

Code:

g_Commands:
    db 'HELP',0
    db 'VER',0
    db 'DATE',0
    db 'TIME',0
    db 'DIR',0
    db 'CLS',0
    db 'REBOOT',0
c_CommandsEnd = $

Command handler table

Code:

g_Handlers:
    dw OnHelp
    dw OnVer
    dw OnDate
    dw OnTime
    dw OnDir
    dw OnCls
    dw OnReboot

This router works perfect

Code:

    .on_cmd:
        mov     bp,g_Handlers                   ; First command handler
        mov     si,g_Commands                   ; SI -> Command string table
    .reset:
        mov     di,g_CmdBuf                     ; DI -> command buffer
    .compare:
        cmp     si,c_CommandsEnd                ; End of command string table?
        je      .not_cmd
        mov     al,byte[si]
        cmp     al,byte[di]                     ; Is same?
        jne     .advance                        ; If no, get next handler
        test    al,al                           ; Is NULL?
        jz      .handle                         ; If yes, invoke it!
        inc     si                              ; Next command char
        inc     di                              ; Next buffer char
        jmp     .compare
    .advance:                                   ; Advances SI past next NULL
        lodsb
        test    al,al
        jnz     .advance
        add     bp,2                            ; Point to next command handler
        jmp     .reset
    .handle:
        jmp     word[bp]                        ; Jump into command handler
    .not_cmd:

FASM complain about jumping

Code:

    .on_cmd:
        mov     bp,g_Handlers                    ; First command handler
        mov     si,g_Commands                   ; SI -> Command string table
    .reset:
        mov     di,g_CmdBuf                     ; DI -> command buffer
    .compare:
        cmp     si,c_CommandsEnd                ; End of command table?
        je      .not_cmd
        mov     al,byte[si]
        cmp     al,byte[di]                     ; Is same?
        jne     .advance                        ; If no, get next handler
        test    al,al                           ; Is NULL?
        jz      word[bp] ; OFFENDING INSTRUCTION
        inc     si                              ; Next command char
        inc     di                              ; Next buffer char
        jmp     .compare
    .advance:                                   ; Advances SI past next NULL
        lodsb
        test    al,al
        jnz     .advance
        add     bp,2                            ; Point to next command handler
        jmp     .reset
    .not_cmd:

What is going on here?

_________________
Coding a 3D game engine with fasm is like trying to eat an elephant,
you just have to keep focused and take it one 'byte' at a time.

07 Jan 2010, 03:18

sinsi

Joined: 10 Aug 2007
Posts: 794
Location: Adelaide

sinsi 07 Jan 2010, 03:37

Maybe because a conditional jump is usually a signed byte relative to IP?
That is one reason older 16-bit code had 'jump islands' for when a jump was too far.

07 Jan 2010, 03:37

DOS386

Joined: 08 Dec 2006
Posts: 1905

DOS386 07 Jan 2010, 14:03

Quote:

Strange Warning
jz word [bp] ; OFFENDING INSTRUCTION
What is going on here?

And the error is ??? Confused

Code:

jz word [bp] ; This is NOT valid since conditional jumps accept only
jz near [bp] ; constant relative to IP (only 8 bit 8086, also 16 or 32 bits since 80386)

better 8086-compatible way:

Code:

jnz short @f
jmp near [bp]
@@:

EDIT : fixed BUG pointed below, see below for better solutions

Last edited by DOS386 on 08 Jan 2010, 08:42; edited 1 time in total

07 Jan 2010, 14:03

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 07 Jan 2010, 17:00

It won't work even on Core i7, as sinsi said, conditional jump instructions only accept relative operand (the assembler transforms a label reference or a number into a number relative to ({R,E})IP.

07 Jan 2010, 17:00

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 05:39

I was unaware of that, thanks for the tips Smile

So does anyone have an idea on how to improve this algorithm?
I thought about length prefixing so i could rep cmpsb...
Maybe you have an interesting idea or better approach?

08 Jan 2010, 05:39

edfed

Joined: 20 Feb 2006
Posts: 4353
Location: Now

edfed 08 Jan 2010, 05:48

Code:

mov bp,handler

...
jz .call
...


.call:
call near[bp]

08 Jan 2010, 05:48

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 05:51

I cant use call since each handler jmp's back into the main loop.
Im trying to keep is small and fast at the same time...

08 Jan 2010, 05:51

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jan 2010, 06:42

WARNING: Completely untested

Code:

g_Commands:
    db 4, 'HELP',   OnHelp
    db 3, 'VER',    OnVer
    db 4, 'DATE',   OnDate
    db 4, 'TIME',   OnTime
    db 3, 'DIR',    OnDir
    db 3, 'CLS',    OnCls
    db 6, 'REBOOT', OnReboot
    db 0
.
.
.
    .on_cmd:
        mov     si, g_Commands - 2
        xor     cx, cx

    .compare:
        add     si, cx
        add     si, 2
        lodsb
        test    al, al
        jnz     .not_cmd

        mov     cl, al
        repe    cmpsb
        jne     .compare

        jmp     word [si]


    .not_cmd:
.
.
.

Not one of the fastest methods for sure, but I think this kind of code, where the human can't notice any slowdown even if it takes several milliseconds, deserves to be optimized for size rather than speed. (Note however that I'm not assuring you that this is the smaller you can do)

08 Jan 2010, 06:42

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 07:46

I never thought of merging the strings and handlers, great idea!
Its so much easier to look at it that way.
My goal is to have an easy way to add new functionality.

08 Jan 2010, 07:46

sinsi

Joined: 10 Aug 2007
Posts: 794
Location: Adelaide

sinsi 08 Jan 2010, 08:10

Just a small note about the original code - accessing [bp] uses the SS segment, not DS, so word[bp] is actually word[ss:bp].
If ss <> ds you will run into problems.

08 Jan 2010, 08:10

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 08:46

Thanks sinsi Smile

Ok, after some changes, Loco's code now works...

First, fasm was complaining about size's so i changed the command table to...

Code:

    g_Commands:
        db 4,'HELP'
        dw OnHelp
        db 3,'VER'
        dw OnVer
        db 4,'DATE'
        dw OnDate
        db 4,'TIME'
        dw OnTime
        db 3,'DIR'
        dw OnDir
        db 3,'CLS'
        dw OnCls
        db 6,'REBOOT'
        dw OnReboot
        db 0

Then the DI register was getting trashed by cmpsb so i fixed that.
And also the code allowed HELP*** to be legal so i fixed that too.
(The command input buffer is NULL terminated after being filled)

Code:

    .on_cmd:
        mov     si,g_Commands - 2
        xor     cx,cx

    .compare:
        add     si,cx
        add     si,2
        lodsb
        test    al,al
        jz      .NotCmd

        mov     di,g_CmdBuf          ; REFRESH DI
        mov     cl,al
        repe    cmpsb
        jnz     .compare             ; CHECK ZF INSTEAD
        cmp     byte[di],0           ; VERIFY END OF COMMAND
        jne     .compare

        jmp     word[si]
    .not_cmd:

Its fun exploring the different ways to skin a cat Smile

Thanks for your help guys!

08 Jan 2010, 08:46

sinsi

Joined: 10 Aug 2007
Posts: 794
Location: Adelaide

sinsi 08 Jan 2010, 09:02

If you know the length of the input buffer that is another way to skip comparing (if length <> commandlength).
Do you convert your input to upper case?

08 Jan 2010, 09:02

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 09:09

Yes, it gets converted to UPPERCASE.
But the length is never saved...

I acquire input into buffer by DI
Then NULL terminate it upon <ENTER>
Then to uppercase (in reverse)
Then goes to command router...

08 Jan 2010, 09:09

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jan 2010, 16:30

bitshifter, jne=jnz.

Code:

jne $
jnz $

load a byte from $-4
load b byte from $-2

if a = b
  display "Same opcode!", 13, 10
else
  display "Not the same instruction", 13, 10
end if

What incredibly stupid mistakes I made there...

08 Jan 2010, 16:30

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 18:09

I think that by using

Code:

rep     cmpsb
jne     .compare

We can also eliminate

Code:

add     si,cx

And then let cmpsb burn through the whole thing...
But this is just an early morning observation...

08 Jan 2010, 18:09

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jan 2010, 18:37

Code:

rep
repe

load a byte from $-2
load b byte from $-1

if a = b
  display "Same opcode!", 13, 10
else
  display "Not the same instruction", 13, 10
end if

08 Jan 2010, 18:37

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jan 2010, 18:58

Hmm, although it says they are same opcode
my changes doesnt work like i thought it would Sad

Code:

rep cmpsb

Not same as!

Code:

repe cmpsb

I need some coffee before my brain starts functioning properly...

08 Jan 2010, 18:58

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jan 2010, 19:10

Just to be sure I've just tested "rep cmpsb" and "repe cmpsb" and both have the very same opcode+prefix code (F3 A6).

With the same I mean that with REP you won't get the "run until CX is zero" effect but "run until CX is zero or ZF is non-zero (flag checked after comparison)". CMPS accepts REPE (which has the same opcode than REP) and REPNE only.

08 Jan 2010, 19:10

DOS386

Joined: 08 Dec 2006
Posts: 1905

DOS386 09 Jan 2010, 02:11

> Strange Warning
> FASM complain about jumping

There is no "Strange Warning" at all, just a justified complaint about faulty code. Also FASM IIRC doesn't support warns at all, just errors Wink

Also it finds per compilation attempt just one compiler-detectable bug, even if there are more (see shot), and this is not necessarily the earliest bug in the source (see shot, try to comment-out line 12).

Description:
Filesize:	2.97 KB
Viewed:	13431 Time(s)

09 Jan 2010, 02:11

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 09 Jan 2010, 04:53

Thanks DOS386 Smile

Ok, so now that it works nicely, the next step is to
make it more readable, thus we introduce a new macro!

So i open the FASM manual and read more about macro's.
Then i compile the LISTING.EXE tool included with FASM.
Now i can see exactly what FASM is making with my macro.
(Good thing, because my first few tries were way off)

So in the long run i came up with this macro...

Code:

macro MESSAGE_MAP command*,handler* {
  local marker
  db marker - $ - 1     ; prefixed length
  db command            ; command string
marker:
  dw handler            ; command handler
}

Now i tested its output with LISTING.EXE and all is good Smile

So now unreadable code like this...

Code:

    g_Commands:
        db 4,'HELP'
        dw OnHelp
        db 3,'VER'
        dw OnVer
        db 4,'DATE'
        dw OnDate
        db 4,'TIME'
        dw OnTime
        db 3,'DIR'
        dw OnDir
        db 3,'CLS'
        dw OnCls
        db 6,'REBOOT'
        dw OnReboot
       db 0

Now becomes readable once again like this...

Code:

    g_Commands:
        MESSAGE_MAP 'HELP',   OnHelp
        MESSAGE_MAP 'VER',    OnVer
        MESSAGE_MAP 'DATE',   OnDate
        MESSAGE_MAP 'TIME',   OnTime
        MESSAGE_MAP 'DIR',    OnDir
        MESSAGE_MAP 'CLS',    OnCls
        MESSAGE_MAP 'REBOOT', OnReboot
     db 0

Ahh, good times, the adventures of coding with FASM Smile

09 Jan 2010, 04:53

< Last Thread | Next Thread >

Forum Rules:

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You can download files in this forum