bug in arralloc / arralloc$()

[qWord]

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.

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

[dedndave]

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]

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

[dedndave]

   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]

Oops, you are right, the pop must come before the jz :redface:

[dedndave]

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

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.

[jj2007]

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]

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]

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

[hutch--]

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.

[dedndave]

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]

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.

[dedndave]

you should think out of the box ;)

 
giving you a hard time - lol

[dedndave]

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