FindBugs for Assembler

[jj2007]

Hi folks,

Following a debate in the Is there static analysis tools for x86 assembler? 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

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...

[Antariy]

Sounds like great tool, Jochen :t

[anta40]

I tried bin2b_ex.asm (the biggest file in masm32 lib source code), and the output was:

Code Select Expand


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


Let's see if I could find bigger source code 

[Gunther]

Jochen,

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

Gunther

[jj2007]

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 

Version 2 attached. Does somebody have a nice source in the 10,000+ lines range for testing?

[dedndave]

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

Code Select Expand

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

[jj2007]

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...

[japheth]

Good tool!

Btw, one can also make Masm detect unused locals with option -W3.

[jj2007]

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 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.

[FORTRANS]

Hi,

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.

[sinsi]

Invoke hater? Heh.

Code Select Expand


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

[dedndave]

i sent you an e-mail with source, Jochen

[jj2007]

   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:

[sinsi]

Always happy to be of service 

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

[jj2007]

You have been warned for your lingo-istic coding style 8)

    push eax
    jmp ExitProcess