The MASM Forum

General => The Workshop => Topic started by: jj2007 on June 07, 2013, 01:17:16 AM

Title: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 01:17:16 AM
Hi folks,

Following a debate in the Is there static analysis tools for x86 assembler? (http://masm32.com/board/index.php?topic=1984.0) thread, I have hacked together a proggie that scans code for little bugs here and there.

Usage is simple: Just drag a source over the executable. Applied to my RichMasm source, it spits out something like this (shortened):

Line 1807       WndProc
                Stack: push-pop=3, correction for calls=7  <<<<<<<<< invoke haters at work :biggrin:

Line 3899       UserMenus
                Stack: push-pop=2

Line 5522       CbTimer
                LOCAL hEdit unused
                LOCAL hOK unused
                LOCAL hCancel unused
                LOCAL rc unused
                LOCAL rcp unused
                Line 5578:      chrg not initialised:   mov eax, txrg.chrg.cpMax

Line 13974      CopyPlainText
                LOCAL cptNumBytes unused
                LOCAL cptClip unused
                Stack: push-pop=1

X:\Masm32\RichMasm\RichMasm.asm: 40 warnings found in 14689 lines


Source attached (FindAsmBugs.asc, RTF format). Would like to see how it performs for your big sources... and of course, the usual disclaimers apply, assembler is not for kids and use at your own risk etc bla bla...
Title: Re: FindBugs for Assembler
Post by: Antariy on June 07, 2013, 01:23:27 AM
Sounds like great tool, Jochen :t
Title: Re: FindBugs for Assembler
Post by: anta40 on June 07, 2013, 01:28:31 AM
I tried bin2b_ex.asm (the biggest file in masm32 lib source code), and the output was:

..\m32lib\bin2b_ex.asm: 0 warnings found in 2326 lines


Let's see if I could find bigger source code  :biggrin:
Title: Re: FindBugs for Assembler
Post by: Gunther on June 07, 2013, 06:35:20 AM
Jochen,

seems to be a good idea. I like especially the invoke haters.  :lol:

Gunther
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 07:18:15 AM
Quote from: Gunther on June 07, 2013, 06:35:20 AM
I like especially the invoke haters.  :lol:

They are causing some headaches, actually. Take this case:
  push edi
  push ebx
  push ecx
  push [esp+20]
  call MbGetSlotPointer                ; needs only one arg, not four!


There is a dedicated invoke hater loop that assumes pushes shortly before a call are arguments. Well, not here - and uses edi ebx ecx does not work for PROLOGUE:NONE.

So, expect some false positives  :biggrin:

Version 2 attached. Does somebody have a nice source in the 10,000+ lines range for testing?
Title: Re: FindBugs for Assembler
Post by: dedndave on June 07, 2013, 09:29:25 AM
that's actually pretty good, considering i sometimes use LEAVE to balance the stack,
and i often push my own parms (sometimes inserting other functions)   :P

the program has no known bugs, by the way
Line 469        MsgLoop  (28 lines)
                Stack: push-pop=-1

Line 506        pCalc    (85 lines)
                Stack: push-pop=1

Line 603        TxtProc  (193 lines)
                Stack: push-pop=14, correction for calls=11

Line 808        WndProc  (884 lines)
                Stack: push-pop=-16, correction for calls=13

Line 1735       CtlUpdate        (410 lines)
                Stack: push-pop=32, correction for calls=1

Line 2208       SndData  (94 lines)
                Stack: push-pop=8

Line 2312       SndCtrl  (26 lines)
                Stack: push-pop=7

Line 2348       AckThrd  (26 lines)
                Stack: push-pop=7

Line 2384       SndAck   (8 lines)
                Stack: push-pop=2

Line 2613       MakeFont         (19 lines)
                Stack: push-pop=-1, correction for calls=1

Line 2689       StMet    (18 lines)
                Stack: push-pop=-4

Line 2752       WkMet    (17 lines)
                Stack: push-pop=-4

Line 2870       TestComplete     (38 lines)
                Stack: push-pop=8

Line 3002       dUpdate  (92 lines)
                Stack: push-pop=1

Line 3284       dTxtOut  (265 lines)
                Stack: push-pop=-4, correction for calls=1

Line 3625       FSaveAs  (135 lines)
                Stack: push-pop=22

Line 3764       CColor   (66 lines)
                Stack: push-pop=8

Line 4055       HFree    (10 lines)
                Stack: push-pop=2

Line 4072       ScnLen   (52 lines)
                Stack: push-pop=-1

Line 4128       Scn2File         (59 lines)
                Stack: push-pop=-1

Line 4194       OFNProc  (114 lines)
                Stack: push-pop=-5

Line 4319       CBTProc  (32 lines)
                Stack: push-pop=3

Line 4358       dWinExit         (121 lines)
                Stack: push-pop=2

Line 4486       B2Hex    (29 lines)
                Stack: push-pop=-1

Line 4522       wmCreate         (54 lines)
                Stack: push-pop=6, correction for calls=5

Line 4583       DeRef    (232 lines)
                Stack: push-pop=4

Line 4825       EnumFntCB        (45 lines)
                Stack: push-pop=1

Line 4877       RegTwo   (47 lines)
                Stack: push-pop=11

Line 4928       _main    (669 lines)
                Stack: push-pop=-2, correction for calls=6

Line 5606       OpnPres  (97 lines)
                Stack: push-pop=2

Line 5743       StaticTest       (103 lines)
                Stack: push-pop=-1
41 ms
dtest.asm: 31 warnings found in 5877 lines
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 03:39:14 PM
Dave,
Thanks. Could you post some of the short ones, in order to see why we got warnings? I'd like to reduce the # of warnings...
Title: Re: FindBugs for Assembler
Post by: japheth on June 07, 2013, 05:15:56 PM

Good tool!

Btw, one can also make Masm detect unused locals with option -W3.
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 06:13:32 PM
Quote from: japheth on June 07, 2013, 05:15:56 PM
Btw, one can also make Masm detect unused locals with option -W3.

Thanks, good to know. However, it spits out loads of often irrelevant warnings, the line numbers point to endp, not to the LOCALs section, and it does not spot the real problem, i.e. reading from garbage:

WinMain proc hInst:DWORD, hPrevInst:DWORD, CmdLine:DWORD, CmdShow:DWORD
LOCAL wc:WNDCLASSEX, msg:MSG, NotRef
   mov eax, wc.cbSize  ; bad idea!


tmp_file.asm(1795) : warning A6004:procedure argument or local not referenced : CmdLine
tmp_file.asm(1795) : warning A6004:procedure argument or local not referenced : CmdShow
tmp_file.asm(1795) : warning A6004:procedure argument or local not referenced : NotRef

FindAsmBugs would show this *):

Line 1706       WinMain  (88 lines)
                Line 1711:      wc not initialised:     mov eax, wc.cbSize
                LOCAL NotRef unused


... and it would not complain if the first usage of wc would be an assignment of some sort.

The problem with compiler/assembler warnings is that if there are too many, the really important ones get lost in the noise. For example, I have a C source that uses parts of a GNU library:

Writing debug information
Compacting CodeView information
9   warnings 'is longer than 8 characters; truncated'
1396   warnings 'missing contents flag; assuming DATA'


The last two lines result from a filter I had to write (http://forum.pellesc.de/index.php?topic=5133.0) because 110k, i.e.  1,400 lines of warnings were just too much:
POLINK: warning: Section '.debug_abbrev' is missing contents flag; assuming DATA.
POLINK: warning: Section '.debug_info' is missing contents flag; assuming DATA.
...
POLINK: warning: Section '.debug_frame' is missing contents flag; assuming DATA.
POLINK: warning: Section '.debug_loc' is missing contents flag; assuming DATA.


Anyway, grateful for more feedback especially on false positives.

*): Attached version three highlights (red on white) the warnings that contain access to uninitialised variables.
Title: Re: FindBugs for Assembler
Post by: FORTRANS on June 07, 2013, 09:35:00 PM
Hi,

Quote from: jj2007 on June 07, 2013, 07:18:15 AM
Does somebody have a nice source in the 10,000+ lines range for testing?



Line 8179    Move_Screen0    (35 lines)
      Stack: push-pop=-2

Line 8220    Move_Screen1    (46 lines)
      Stack: push-pop=-2

Line 9099    LINE06    (67 lines)
      Stack: push-pop=1

Line 9170    VertLine06    (24 lines)
      Stack: push-pop=-1

Line 9196    HorzLine06    (23 lines)
      Stack: push-pop=-1

Line 9541    bin2dec    (48 lines)
      Stack: push-pop=1

Line 10505    ELLIPSE06    (193 lines)
      Stack: push-pop=-1
1735 ms
draw-lx.asm : 7 warnings found in 10852 lines


   Very nice.  I had (more or less) expected garbage (or null) from a
MASM 5.0 DOS program.  Kudos.

Cheers,

Steve N.
Title: Re: FindBugs for Assembler
Post by: sinsi on June 07, 2013, 10:02:04 PM
Invoke hater? Heh.

start proc public
    local m:MSG
    sub ebx,ebx

    push ebx
    call GetModuleHandle
    mov hinst,eax

    push ebx                ;hIconSm
    push OFFSET frmclass    ;lpszClassName
    push ebx                ;lpszMenuName
    push COLOR_WINDOW+1     ;hbrBackground
    push 32512
    push ebx
    call LoadCursor
    push eax                ;hCursor
    push 32512
    push ebx
    call LoadIcon
    push eax                ;hIcon
    push hinst              ;hInstance
    push ebx                ;cbWndExtra
    push ebx                ;cbClsExtra
    push OFFSET frmproc     ;lpfnWndProc
    push 3                  ;style
    push 48                 ;cbSize
    push esp
    call RegisterClassEx
    add esp,48
    test eax,eax
    jz done

    push ebx                ;lpParam
    push hinst              ;hInstance
    push ebx                ;hMenu
    push ebx                ;hwndParent
    push 100                ;nHeight
    push 200                ;nWidth
    push ebx                ;Y
    push ebx                ;X
    push frmstyle
    push OFFSET frmtitle    ;lpWindowName
    push OFFSET frmclass    ;lpClassName
    push ebx                ;dwExStyle
    call CreateWindowEx
    test eax,eax
    jz done

    lea esi,m
  next:
    INVOKE GetMessage,esi,ebx,ebx,ebx
    test eax,eax
    jz done
    INVOKE TranslateMessage,esi
    INVOKE DispatchMessage,esi
    jmp next
  done:
    push eax
    jmp ExitProcess
start ENDP



Line 0  start    (59 lines)
                Stack: push-pop=-8, correction for calls=39
0 ms
F:\asm\asm\window\jj.asm: 1 warnings found in 60 lines

Title: Re: FindBugs for Assembler
Post by: dedndave on June 07, 2013, 10:31:36 PM
i sent you an e-mail with source, Jochen
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 10:48:17 PM
Quote from: FORTRANS on June 07, 2013, 09:35:00 PM
   Very nice.

Thanks, Steve. Could you post a few short examples where it issued warnings, so that I can see if they legit or not?

@Sinsi: Thanks, you pointed me to an obvious flaw. Attached the new version, it will bang you over the head exclusively for the last jmp :icon_mrgreen:
Title: Re: FindBugs for Assembler
Post by: sinsi on June 07, 2013, 11:09:28 PM
Always happy to be of service  :biggrin:

Line 0  start    (59 lines)
                Stack: push-pop=1, correction for calls=30
0 ms
F:\asm\asm\window\jj.asm: 1 warnings found in 60 lines
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 07, 2013, 11:21:03 PM
You have been warned for your lingo-istic coding style 8)

    push eax
    jmp ExitProcess

Title: Re: FindBugs for Assembler
Post by: sinsi on June 07, 2013, 11:29:18 PM
Yeah, it should be "ret"  :P
Title: Re: FindBugs for Assembler
Post by: Antariy on June 07, 2013, 11:35:40 PM
Quote from: jj2007 on June 07, 2013, 10:48:17 PM

@Sinsi: Thanks, you pointed me to an obvious flaw. Attached the new version, it will bang you over the head exclusively for the last jmp :icon_mrgreen:

Yeah! It should be "push eax \ push eax \ jmp ExitProcess" if code wants to return something from eax! :biggrin:

Quote from: jj2007 on June 07, 2013, 11:21:03 PM
You have been warned for your lingo-istic coding style 8)

    push eax
    jmp ExitProcess



I afraid lingo was walking afoot under the table at the time when that coding style was first used :greensml:
Title: Re: FindBugs for Assembler
Post by: sinsi on June 07, 2013, 11:41:11 PM
Quote from: Antariy on June 07, 2013, 11:35:40 PM
Yeah! It should be "push eax \ push eax \ jmp ExitProcess" if code wants to return something from eax! :biggrin:
Yes, push/call is different to push/jmp :icon_redface:

OTOH, I don't use the return code anyway  :biggrin:
Title: Re: FindBugs for Assembler
Post by: Antariy on June 08, 2013, 12:06:05 AM
Quote from: sinsi on June 07, 2013, 11:41:11 PM
OTOH, I don't use the return code anyway  :biggrin:

Then it is OK :t (psss... I did the same in the MemInfoMicro25 :biggrin:)
Title: Re: FindBugs for Assembler
Post by: Antariy on June 08, 2013, 01:04:00 AM
Jochen, as far as I tested, your latest program complains only about advanced push/call sequences. The messages are like this:


Line 55         btnproc  (80 lines)
                Stack: push-pop=5, correction for calls=6


That's the AxMsgTableViewer (you remember it from the old forum) "neat-buttons" subclassing code.
I'll post the actual code here (http://masm32.com/board/index.php?topic=2004.msg20946#msg20946) if someone will find it useful or interesting.
You'll see why that code causes warnings :lol: But actually that's hard to analyse code and require much and much recursive processing, so I think it's OK.

Title: Re: FindBugs for Assembler
Post by: FORTRANS on June 08, 2013, 02:59:17 AM
Hi Jochen,

   I hope this is enough?


; - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
; MOVE_SCREEN - Move bytes at DS:[SI] [WORK1] to the screen at ES:[DI] 00
; PSEUDO.ASM 1986 for Z-100
; PSEUDO2.ASM 11 May 1999 for HPLX - CGA
; Code modified for 200LX DRAW-LX 29 July 2002, SRN (MOVSW)

        PUBLIC  Move_Screen0    ; move from work area to screen
Move_Screen0    PROC
        ASSUME  CS:CODE
        MOV     AX,0B800H       ; Point to CGA video memory
        MOV     ES,AX
        XOR     DI,DI
        MOV     SI,[WORK1]      ; make offsets

        CLD
        MOV     CX,100-(199-dispYMax)/2 ; count 100 records (odd lines per screen)

MS0_1:  PUSH    CX

        MOV     CX,40           ; count 40 words (80 bytes per line)
                                ; = 640 monochrome pixels
    REP MOVSW                   ; Data seg = Work, Extra seg = screen

        ADD     SI,050H         ; Skip line in work area so odd lines copied
        POP     CX
        LOOP    MS0_1
; - - even lines - -
        MOV     SI,[WORK1]
        ADD     SI,050H         ; Point to second line, CGA lines use 80 bytes
        MOV     DI,02000H       ; 100 line offset + waste for CGA screen area
        MOV     CX,100-(199-dispYMax)/2          ; count 100 records (even lines per screen)

MS0_3:  PUSH    CX

        MOV     CX,40           ; count 40 words (bytes per line)

    REP MOVSW

        ADD     SI,050H         ; Skip line in work area to copy even lines
        POP     CX
        LOOP    MS0_3
        RET
Move_Screen0    ENDP

...

; - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
; If we can't get their routines to work, we'll make a
; bog simple line routine, 9 August 2002, SRN
VertLine06      PROC    NEAR

        MOV     AX,[y1]         ; AX = y1
        MOV     CX,[y2]
        SUB     CX,AX           ; CX = y2-y1
        JGE     VL_1            ; jump if dY >= 0

        ADD     AX,CX           ; AX = y2
        NEG     CX              ; force dY >= 0
VL_1:   INC     CX              ; CX = # of pixels to draw

; point to the Topmost pixel (??) Smallest line #
        MOV     [usY],AX
        MOV     AX,[x1]
        MOV     [usX],AX

; draw the line
VL_2:   PUSH    CX              ; preserve CX
        CALL [SetPixel]
        INC     [usY]
        POP     CX              ; CX = # of pixels
        LOOP    VL_2

        RET
VertLine06      ENDP

HorzLine06      PROC    NEAR

; ensure that x1 < x2 (move left to right)
        MOV     AX,[x1]         ; AX = x1
        MOV     CX,[x2]         ; CX = x2
        SUB     CX,AX           ; CX = x2-x1
        JAE     HL_1

        ADD     AX,CX           ; AX = x2
        NEG     CX              ; force dX >= 0

HL_1:   INC     CX              ; CX = # of pixels to draw
        MOV     [usX],AX
        MOV     AX,[y1]
        MOV     [usY],AX

HL_2:   PUSH    CX              ; preserve CX
        CALL [SetPixel]
        INC     [usX]           ; Move right
        POP     CX              ; CX = # of pixels
        LOOP    HL_2
        RET

HorzLine06      ENDP


Regards,

Steve N.
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 08, 2013, 07:34:51 AM
Hi Steve,

That was really helpful, thanks :t

It was the  MyLabel: push whatever  that caused the trouble. With version 4 (attached), you'll get zero warnings.
Title: Re: FindBugs for Assembler
Post by: Antariy on June 08, 2013, 08:58:02 AM
Jochen, I tested it with couple other more or less big projects, and all warnings I get are about the same - "stack".

I think you may potentially solve even this cases in such a way.

Code collects all prototypes of APIs in the include dir of MASM32 distribution.
Then it, for the push/pop/calls correctness checking do the down-up walking through the proc, and having the number of params for every API that we get in the previous step, it's possible to count and correspond every push to every API in a intermixed push/call/push/call sequences.
Having the calling convention from proto we have also info about how much stack should be free after a call. But this is more complicated to account, because, for an instance, for a Cdecl there is possible "accamulated" or "delayed" freeing, or reusage of previously pushed params.
Accamulated freeing is something like:

push CTXT("Press Y/N",10)
call crt_printf
invoke crt__getche
or eax,20h
cmp eax,"y"
mov eax, CTXT("Yes pressed",10)
jz @F
mov eax,CTXT("No pressed",10)
@@:
push eax
call crt_printf
add esp,8 ; the sum of two params for two calls


Reusage may be even more unpredictable and hard to find, one of the examples (just from the code I posted in prologue thread - TestStack.asm):

   push 0
   invoke RecursiveFunction,lpVoid,esp
   
   mov eax,lpVoid
   mov eax,[eax]
   invoke crt_printf,CTXT("The thread #%u do %u recursive calls (limited to the stack size only)",10),eax
   lock dec dword ptr numThreads
   pop edx


Take notice that invoke is used with an implicit param that even did not lie near of the invoke :biggrin:
Actually such things with cdecl may be quite complicated and efficient.

From mw.asm (timeslice thread):

    add esp,-16

    invoke crt_printf,CTXT("Calculating CPU speed... ")
    xor eax,eax
    cpuid
    rdtsc
    xchg esi,eax
    mov edi,edx
    invoke GetTickCount
    xchg eax,ebx
    neg ebx
    invoke Sleep,2000
    invoke GetTickCount
    add ebx,eax
    push ebx
    xor eax,eax
    cpuid
    rdtsc
    sub eax,esi
    sbb edx,edi
    shrd eax,edx,1
    shr edx,1
    mov [esp+4],eax
    mov [esp+8],edx
    invoke crt_printf,CTXT("(Sleep(2000) takes: %d): ~%I64u Hz",10)
    pop ecx       
   

    fild qword ptr [esp]
    mov dword ptr [esp],1000000
    fild dword ptr [esp]
    fdivp st(1),st(0)
       
   
    xor edi, edi
    mov esi, pBuff
  @@:
    fild qword ptr [esi+edi+12+4]
    fild qword ptr [esi+edi+4]
    fsubp st(1),st(0)
    fld st(0)
    fistp qword ptr [esp]
    fdiv st(0),st(1)
    fstp qword ptr [esp+8]
    push [esi+edi+8]
    push [esi+edi+4]
    push dword ptr [esi+edi]
    push CTXT("ThrId: %d, Tsc: %I64X (%I64u clocks, %.3lf microseconds)",10)
    call crt_printf
    add esp,4*4
    add edi,12
    cmp edi,(TIMES_TO_SWITCH-1)*THREADS_COUNT*12
    jb @B
   
  @@:
    add esp,16


There are again the params that are pushed just before the call, and params that were placed into the stack before the call to functions, and were re-used after a call, partial stack cleanup to keep those re-used params there.
Title: Re: FindBugs for Assembler
Post by: Antariy on June 08, 2013, 09:05:24 AM
Also, for that file - mw.asm - the results:




Line 108        calc_timeslice   (135 lines)
                Stack: push-pop=6, correction for calls=1
46 ms
C:\masm32\mw.asm: 1 warnings found in 339 lines


Interesting that it found the creation of the structure dynamically on the stack (it seems so - but there are 7 pushes, not 6; the esp is restored from local var), but said nothing about that entangled piece I colored above :biggrin:
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 08, 2013, 04:22:45 PM
Quote from: Antariy on June 08, 2013, 08:58:02 AM
Jochen, I tested it with couple other more or less big projects, and all warnings I get are about the same - "stack".

I think you may potentially solve even this cases in such a way.

Code collects all prototypes of APIs in the include dir of MASM32 distribution.
Then it, for the push/pop/calls correctness checking do the down-up walking through the proc, and having the number of params for every API that we get in the previous step, it's possible to count and correspond every push to every API in a intermixed push/call/push/call sequences.
Having the calling convention from proto we have also info about how much stack should be free after a call. But this is more complicated to account, ...

Hi Alex,

What you write is correct, of course, but it's a different, more professional project. My intention was to provide a tool that catches most of the typical noob bugs, such as using uninitialised local variables, and an unbalanced stack. Of course, the not-so-noob members here are having a lot of fun with shuffling parameters all over the place and misusing the stack etc, but they are not the target audience. Those who don't use invoke and finish a proc with jmp know what they are doing, right?

So: Apologies if I concentrate on a) catching bugs in "normal" code b) minimise warnings to the really essential ones :icon14:

P.S.: You certainly see the bug chasing potential of the first line of the MyEpilogue macro ;-)
Title: Re: FindBugs for Assembler
Post by: FORTRANS on June 08, 2013, 10:18:06 PM
Quote from: jj2007 on June 08, 2013, 07:34:51 AM
Hi Steve,

That was really helpful, thanks :t

Hi Jochen,

   Glad to help.

Quote
It was the  MyLabel: push whatever  that caused the trouble.

   Odd.  Code on a line with a label is uncommon?

QuoteWith version 4 (attached), you'll get zero warnings.

   Well, not quite.


Line 1193    START    (64 lines)
      Stack: push-pop=1, correction for calls=-1

Line 9541    bin2dec    (48 lines)
      Stack: push-pop=1

Line 10505    ELLIPSE06    (193 lines)
      Stack: push-pop=2, correction for calls=-2
209 ms
draw-lx.asm : 3 warnings found in 10852 lines


   I assume the "correction for calls" means you know what is
going on?  Anyway as the warning for the bin2dec routine did not
change, I am posting that code.  That is copied, by hand,  from "The
Waite Group's MS-DOS Developer's Guide", 2nd ed., if that means
anything.  With trivial modifications for the DOS functions.

IF DEBUG OR DEBUG1 OR DEBUG2
; ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
; BIN2DEC - BINary to DECimal conversion.  Displays a 16-bit
; signed or unsigned number on the screen in decimal.
; Finds the rightmost digit by division.  Repeat until all found.
; A minimum number of digits to be displayed can be specified:
; if minimum number of digits specified is greater than the
; actual number of digits, the output number is padded with
; leading zeros.

; INPUT:         AX = number to be displayed
;                CH = minimum number of digits to be displayed
;                DX = 0 if number is to be processed as unsigned,
;                     or 1, if signed.

; OUTPUT:        None (AX, CX, and DX are restored)

; ROUTINES CALLED: None

PUBLIC  BIN2DEC                 ; library routine

bin2dec PROC    NEAR
        push    ax              ; save registers
        push    bx
        push    cx
        push    dx
        mov     cl,0            ; clear digit count
        mov     bx,10           ; set divisor = 10
        cmp     dx,0            ; always display # as positive?
        je      more_dec        ; yes, skip negative check

; Check for negative number.  If negative, make number positive.
        or      ax,ax           ; is number positive?
        jnl     more_dec        ; yes, skip "negate"
        neg     ax              ; make number positive
        MOV     DL,'-'             ; display minus sign
        SCALL CONOUT

; Main Division Loop - Get Decimal Digit
; Repeat as long as digits are remaining
more_dec:
        xor     dx,dx           ; cleanup
        div     bx              ; divide by 10
        push    dx              ; save remainder
        inc     cl              ; digit counter + 1
        or      ax,ax           ; test quotient
        jnz     more_dec        ; continue if more

; Main Digit Print Loop - Reverse Order
        sub     ch,cl           ; min. number of digits reached?
        jle     morechr         ; yes - begin display
        xor     dx,dx           ; no - start pushing "O"s
morezero:
        push    dx
        inc     cl              ; digit counter + 1
        dec     ch              ; check if matched yet
        jnz     morezero        ; no - keep pushing it
morechr:
        pop     dx              ; restore last digit
        add     dl,30h          ; convert to ASCII
        SCALL   CONOUT          ; output digit
        dec     cl              ; digits count - 1
        jnz     morechr         ; continue if more

        pop     dx              ; restore registers
        pop     cx
        pop     bx
        pop     ax
        ret
bin2dec ENDP
ENDIF


Regards,

Steve N.
Title: Re: FindBugs for Assembler
Post by: Antariy on June 08, 2013, 11:04:39 PM
Quote from: jj2007 on June 08, 2013, 04:22:45 PM
Quote from: Antariy on June 08, 2013, 08:58:02 AM
Jochen, I tested it with couple other more or less big projects, and all warnings I get are about the same - "stack".

I think you may potentially solve even this cases in such a way.

Code collects all prototypes of APIs in the include dir of MASM32 distribution.
Then it, for the push/pop/calls correctness checking do the down-up walking through the proc, and having the number of params for every API that we get in the previous step, it's possible to count and correspond every push to every API in a intermixed push/call/push/call sequences.
Having the calling convention from proto we have also info about how much stack should be free after a call. But this is more complicated to account, ...

Hi Alex,

What you write is correct, of course, but it's a different, more professional project. My intention was to provide a tool that catches most of the typical noob bugs, such as using uninitialised local variables, and an unbalanced stack. Of course, the not-so-noob members here are having a lot of fun with shuffling parameters all over the place and misusing the stack etc, but they are not the target audience. Those who don't use invoke and finish a proc with jmp know what they are doing, right?

So: Apologies if I concentrate on a) catching bugs in "normal" code b) minimise warnings to the really essential ones :icon14:

P.S.: You certainly see the bug chasing potential of the first line of the MyEpilogue macro ;-)

Yes, of course I understand your points, Jochen! And you're right :t
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 09, 2013, 12:11:49 AM
Quote from: FORTRANS on June 08, 2013, 10:18:06 PM
QuoteIt was the  MyLabel: push whatever  that caused the trouble.
Odd.  Code on a line with a label is uncommon?
Surely not, it's about as common as Work in Progress with Bugs ;-)

QuoteWith version 4 (attached), you'll get zero warnings.
Quote
   Well, not quite.

What I meant is you should get zero warnings for the MOVE_SCREEN code posted above in reply #20.

I've tried to include some more common cases, but of course there will still be some false positives, especially for hand-crafted code fumbling with frames and stack. Version 5 is attached - drag MoveScreenSteve.asm over the exe to see the result.
Title: Re: FindBugs for Assembler
Post by: FORTRANS on June 10, 2013, 12:10:26 AM
Hello,

QuoteSurely not, it's about as common as Work in Progress with Bugs ;-)

   Aha!  Dim light turning on here.


QuoteI've tried to include some more common cases, but of course there will still be some false positives, especially for hand-crafted code fumbling with frames and stack. Version 5 is attached - drag MoveScreenSteve.asm over the exe to see the result.

   Here are the results:

167 ms
movescre.asm : 0 warnings found in 100 lines

Line 1193    START    (64 lines)
      Stack: push-pop=1, correction for calls=-1

Line 8056    CREATEFL    (114 lines)
      Stack: push-pop=2

Line 9541    bin2dec    (48 lines)
      Stack: push-pop=1

Line 10505    ELLIPSE06    (193 lines)
      Stack: push-pop=2, correction for calls=-2
843 ms
draw-lx.asm : 4 warnings found in 10852 lines


   At least things keep changing.  Color?  What does the colored
warnings signify?  Heck of a macro at least...

Regards,

Steve N.
Title: Re: FindBugs for Assembler
Post by: jj2007 on June 10, 2013, 02:31:13 AM
Quote from: FORTRANS on June 10, 2013, 12:10:26 AM
Color?  What does the colored warnings signify?  Heck of a macro at least...

I try to flag by severity:
Red on white = reading an uninitialised local var
red on normal background = stack 1 or 2 off (smells of bug)
normal colours = unused locals, stack dramatically out of balance (smells of expert code)
:P
Title: Re: FindBugs for Assembler
Post by: FORTRANS on June 10, 2013, 03:38:34 AM
Hi,

   Okay.  Maybe I should look at the program again and try to see
what is being flagged.  The bin2dec and create file routines are OBE
anyway.  Well, have been worked on in other programs since then,
Good old copy, paste, and fix up in a newer program.

Thanks,

Steve N.
Title: Re: FindBugs for Assembler
Post by: Antariy on June 10, 2013, 07:00:57 PM
Quote from: jj2007 on June 10, 2013, 02:31:13 AM
Quote from: FORTRANS on June 10, 2013, 12:10:26 AM
Color?  What does the colored warnings signify?  Heck of a macro at least...

I try to flag by severity:
Red on white = reading an uninitialised local var
red on normal background = stack 1 or 2 off (smells of bug)
normal colours = unused locals, stack dramatically out of balance (smells of expert code)
:P

:biggrin: :lol: :greensml: AHAHAHAHA

...sorry for inarticulate sounds above this line, but that message is just great explanation, could not resist to say that :t Really, it's very witty :biggrin: