Author Topic: bug in arralloc / arralloc$()  (Read 10196 times)

qWord

  • Member
  • *****
  • Posts: 1473
  • The base type of a type is the type itself
    • SmplMath macros
bug in arralloc / arralloc$()
« on: November 04, 2013, 09:37:17 AM »
Running under a debugger, the function arrfree / arrfree$() does cause an exception when freeing the memory of the pointer-array (GlobalFree). The actual problem is the function arralloc/arralloc$(), which does write to the unallocated array element [esi+mcnt*4+4]. Furthermore the loop condition should be checked before accessing the array to avoid problems when mcnt == 0.
Quote
arralloc proc mcnt:DWORD

  ; ----------------------------------------------------------------
  ; return values = handle of pointer array or 0 on allocation error
  ; ----------------------------------------------------------------
    push esi

    mov eax, mcnt                               ; load the member count into EAX
    add eax, 1                                  ; correct for 1 based array
    lea eax, [0+eax*4]                          ; multiply it by 4 for memory size

    invoke GlobalAlloc,GMEM_FIXED,eax
    mov esi, eax

    test eax, eax                               ; if allocation failure return zero
    jz quit

    mov eax, esi
    mov ecx, mcnt
    mov DWORD PTR [eax], ecx                    ; write count to 1st member

    xor edx, edx
  @@:
    add edx, 1                                  ; write adress of null string to all members
    mov [eax+edx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
    cmp edx, ecx
    jle @B <----------- should be JL or JB

    mov eax, esi                                ; return pointer array handle

  quit:
    pop esi

    ret

arralloc endp
MREAL macros - when you need floating point arithmetic while assembling!

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #1 on: November 04, 2013, 10:09:19 AM »
i seem to recall we had troubles with something related a couple years ago

http://www.masmforum.com/board/index.php?topic=17159.0

jj2007

  • Member
  • *****
  • Posts: 10543
  • Assembler is fun ;-)
    • MasmBasic
Re: bug in arralloc / arralloc$()
« Reply #2 on: November 04, 2013, 10:43:22 AM »
It could also be shortened a little bit:

arralloc proc mcnt:DWORD

  ; ----------------------------------------------------------------
  ; return values = handle of pointer array or 0 on allocation error
  ; ----------------------------------------------------------------
    mov eax, mcnt                         ; load the member count into EAX
    push eax
    lea eax, [eax*4+4]                    ; correct for 1 based array, multiply it by 4 for memory size

    invoke GlobalAlloc,GMEM_FIXED,eax

    test eax, eax                        ; if allocation failure return zero
    pop ecx
    jz quit

    mov DWORD PTR [eax], ecx             ; write count to 1st member

  @@:
    mov [eax+ecx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
    dec ecx
    ja @B

; return pointer array handle

  quit:

    ret

arralloc endp
« Last Edit: November 04, 2013, 06:46:35 PM by jj2007 »

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #3 on: November 04, 2013, 11:04:19 AM »
 :biggrin:  a little tidier...
Code: [Select]
    mov     ecx,mcnt
    lea     eax,[4*ecx+4]
    push    ecx
    INVOKE  GlobalAlloc,GMEM_FIXED,eax
    test    eax,eax
    pop     ecx
    jz      quit

jj2007

  • Member
  • *****
  • Posts: 10543
  • Assembler is fun ;-)
    • MasmBasic
Re: bug in arralloc / arralloc$()
« Reply #4 on: November 04, 2013, 11:22:58 AM »
Oops, you are right, the pop must come before the jz :redface:

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #5 on: November 04, 2013, 11:32:50 AM »
well - maybe not - there is always LEAVE
but - it's clearer if you pop first

it wouldn't cost anything to preserve EBX, ESI, or EDI for that value

hutch--

  • Administrator
  • Member
  • ******
  • Posts: 7537
  • Mnemonic Driven API Grinder
    • The MASM32 SDK
Re: bug in arralloc / arralloc$()
« Reply #6 on: November 04, 2013, 01:26:49 PM »
The problem I have had with these algos is that every time someone had a problem you suggested a mod then never got any feedback so I was always left in the lurch as to whether they worked properly.
hutch at movsd dot com
http://www.masm32.com    :biggrin:  :skrewy:

jj2007

  • Member
  • *****
  • Posts: 10543
  • Assembler is fun ;-)
    • MasmBasic
Re: bug in arralloc / arralloc$()
« Reply #7 on: November 04, 2013, 06:48:56 PM »
it wouldn't cost anything to preserve EBX, ESI, or EDI for that value

What's the advantage of using ebx instead of ecx?

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #8 on: November 04, 2013, 09:24:57 PM »
i guess the main advantage is, it's easier to read :)

the cost is - you push/pop EBX (or ESI or EDI) at entry and exit
which is ok - it replaces a push/pop on ECX - so no cost, really
Code: [Select]
arralloc proc uses ebx mcnt:DWORD

  ; ----------------------------------------------------------------
  ; return values = handle of pointer array or 0 on allocation error
  ; ----------------------------------------------------------------
    mov ebx, mcnt                         ; load the member count into EBX
    lea eax, [ebx*4+4]                    ; correct for 1 based array, multiply it by 4 for memory size
    invoke GlobalAlloc,GMEM_FIXED,eax
    test eax, eax                        ; if allocation failure return zero
    jz quit

    mov DWORD PTR [eax], ebx             ; write count to 1st member

  @@:
    mov [eax+ebx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
    dec ebx
    ja @B

; return pointer array handle

  quit:
    ret

arralloc endp

qWord

  • Member
  • *****
  • Posts: 1473
  • The base type of a type is the type itself
    • SmplMath macros
Re: bug in arralloc / arralloc$()
« Reply #9 on: November 06, 2013, 04:12:43 AM »
The problem I have had with these algos is that every time someone had a problem you suggested a mod then never got any feedback so I was always left in the lurch as to whether they worked properly.
to give a feed back now, I would use the following version, which avoids partial flag stalls, access the memory in ascending order and also handle the case mcnt==0:
Code: [Select]
stackframe off
align 16
arralloc2 proc mcnt:DWORD

  ; ----------------------------------------------------------------
  ; return values = handle of pointer array or 0 on allocation error
  ; ----------------------------------------------------------------
    mov eax, [esp+4]                     ; load the member count into EAX
    push eax
    lea eax, [eax*4+4]                   ; correct for 1 based array, multiply it by 4 for memory size
    invoke GlobalAlloc,GMEM_FIXED,eax
    test eax, eax                        ; if allocation failure return zero
    pop edx                              ; load the member count into EDX
    jz quit
     
    mov [eax],edx                        ; write count to 1st member
    and ecx,0                            ; loop alignment
    jmp @1
@@: db 3Eh                               ; DS prefix for alignment
    mov [eax+ecx*4+4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
    add ecx,1
@1: cmp ecx,edx
    jb @B

    ; return pointer array handle

  quit:
    ret 4

arralloc2 endp
stackframe on

Generally, how should one report bugs in the MASM32 SDK? Over the time I've got the impression that a subforum is not that efficient...
MREAL macros - when you need floating point arithmetic while assembling!

hutch--

  • Administrator
  • Member
  • ******
  • Posts: 7537
  • Mnemonic Driven API Grinder
    • The MASM32 SDK
Re: bug in arralloc / arralloc$()
« Reply #10 on: November 06, 2013, 04:38:29 AM »
Its probably the case that I am not always that efficient. I have to find the originals, the test pieces then enough brain space to digest the problem and I don't always have the time. I do have a good look at the posted problems but don't always have the time/brain space to work on it.
hutch at movsd dot com
http://www.masm32.com    :biggrin:  :skrewy:

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #11 on: November 06, 2013, 05:01:57 AM »
as i recall, we had a lot of participation in the old forum on this set of routines
and we resolved the problem as it was reported
but this is a slightly different issue, i think

qWord is a great coder
but........
if they call the function with 0, they get what they deserve   :lol:
aligning short loops yields little advantage, if any

qWord

  • Member
  • *****
  • Posts: 1473
  • The base type of a type is the type itself
    • SmplMath macros
Re: bug in arralloc / arralloc$()
« Reply #12 on: November 06, 2013, 06:02:05 AM »
if they call the function with 0, they get what they deserve   :lol:
you should think out of the box ;)

aligning short loops yields little advantage, if any
the cost are minimal in this case, thus we should use that potential.
MREAL macros - when you need floating point arithmetic while assembling!

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #13 on: November 06, 2013, 06:10:06 AM »
you should think out of the box ;)

 :biggrin:
giving you a hard time - lol

dedndave

  • Member
  • *****
  • Posts: 8827
  • Still using Abacus 2.0
    • DednDave
Re: bug in arralloc / arralloc$()
« Reply #14 on: November 06, 2013, 06:47:54 AM »
well - to be fair.....
if you are going to test for 0, do it before allocating any space   :P
Code: [Select]
arralloc proc uses ebx mcnt:DWORD

  ; ----------------------------------------------------------------
  ; return values = handle of pointer array or 0 on allocation error
  ; ----------------------------------------------------------------
    mov ebx, mcnt                         ; load the member count into EBX
    test ebx, ebx
    mov eax, ebx
    jz  quit