ar: shuffle symbol offsets during conversion for 32-bit ar archives

[emaste]

Summary

This is a merge of three commits from my working tree:

commit 363f3194b2f0f229bdd8a2295319252acfb033f8
Author: Ed Maste <[e-mail]>
Date:   Mon Apr 22 09:39:26 2019 -0400

    ar: test for writing 64-bit format only if symbol count is nonzero

    This is a minor simplification; if we do not have any symbols a 32-bit
    (empty) symbol table is sufficient.

    Sponsored by:   The FreeBSD Foundation

commit 01214a87fa0b029b26d19bb018c43e2f6c480b9e
Author: Ed Maste <[e-mail]>
Date:   Mon Apr 22 09:44:12 2019 -0400

    ar: use array notation to access s_so

    This is somewhat more readable than pointer arithmetic.

    Sponsored by:   The FreeBSD Foundation

commit 17c24416b6bd478c947918c8cc94f19255c651e2 (HEAD -> wipbsd.20190326)
Author: Ed Maste <[e-mail]>
Date:   Mon Apr 22 09:53:12 2019 -0400

    ar: shuffle symbol offsets during conversion for 32-bit ar archives

    During processing we maintain symbol offsets in the 64-bit s_so array,
    and when writing the archive convert to 32-bit if no offsets are greater
    than 4GB.  However, this was inefficient as we looped over the array
    twice: first, converting to big endian and second, writing each 32-bit
    value one at a time.  Instead, when writing a 32-bit archive shuffle
    convert symbol data to big endian and shuffle to the beginning of the
    array at the same time.

    Sponsored by:   The FreeBSD Foundation

[emaste]

usr.bin/ar/write.c

679

XXX

[emaste]

oh still an endian bug though,

if (w_sz == sizeof(uint32_t))
        nr = (uint64_t)htobe32((uint32_t)bsdar->s_cnt);
else
        nr = htobe64(bsdar->s_cnt);
write_data(bsdar, a, &nr, w_sz);

[emaste]

Rework writing s_cnt to avoid 32-bit big endian issues

In the 32-bit case previous code did:

unit64_t nr;
nr = (uint64_t)htobe32((uint32_t)bsdar->s_cnt);
write_data(bsdar, a, &nr, sizeof(uint32_t));

which on BE platforms wrote the upper four zero bytes of nr

[aryeeteygerald_rogers.com]

I like the changes.

[emaste]

Rebase after committing ar: test for writing 64-bit format only if symbol count is nonzero and ar: use array notation to access s_so. Fix build, add missing & in &nr32.

[emaste]

Add UB comment in source

[emaste]

usr.bin/ar/write.c

678–679

For reference clang generates this nice asm:

     e10:       8b 3c f2        movl    (%rdx,%rsi,8), %edi
; htobe32(bsdar->s_so[i] + pm_sz);
     e13:       01 cf   addl    %ecx, %edi
     e15:       0f cf   bswapl  %edi
; ((uint32_t *)(bsdar->s_so))[i] =
     e17:       89 3c b2        movl    %edi, (%rdx,%rsi,4)
; for (i = 0; (size_t)i < bsdar->s_cnt; i++)
     e1a:       48 83 c6 01     addq    $1, %rsi
     e1e:       48 39 f0        cmpq    %rsi, %rax
     e21:       75 ed   jne     -19 <write_archive+0xe00>

[cem]

usr.bin/ar/write.c

677

Can i be size_t? It appears to be int, which is why this cast is present, but size_t is definitely more correct.

678–679

As discussed on IRC this is an aliasing violation, but the compiler doesn't seem to take advantage of that and generate surprising code, for now.

Possible non-UB solutions would be using be32enc() (not portable, but easily embedded); (buffered) directly writing out the big endian values; or allocating a 2nd array for the 32-bit compaction result.

[jhibbits]

Works for me, thanks!

[emaste]

usr.bin/ar/write.c

677

Changed in my WIP tree.

678–679

Yes, I think the best way to do it is malloc a new buffer in write_objs of the appropriate size and do not do the in-place modification for either 32- or 64-bit output.