Re: Bug#653653: nbd: FTBFS on sparc: Bus error (core dumped) - FAIL: integrity

[Patrick Baggett <baggett.patrick@gmail.com>]

There are some other problems too:

makebuf() [ln 701] does this:

uint64_t* p = (uint64_t*)buf; 

in this, buf is a char*. This violates the strict aliasing rules that say a char* may alias any type, but not the other way around: uint64_t* may not alias a char*. The difference is subtle: modifying buf[] will cause a reload of p[] values, but modifying p[] will not cause a reload of buf[]. Though a violation, it is probably safe since only one is accessed in this scope.

Also, checkbuf() [ln 717] does this:

char cmp[512];
makebuf(cmp, seq, blknum);

This means 'cmp' is going to be reinterpreted as a uint64_t in makebuf() -- but the char[] base type does guarantee 8-byte alignment, so this may or may not fail. It should probably be declared as:

uint64_t cmp[64]; // 512/8 = 64
makebuf((char*)cmp, seq, blknum);

Changing the protoype of makebuf() to take uint64_t may be appropriate as well since it seems to be called in that context.

Patrick

On Sat, Mar 3, 2012 at 10:11 AM, Julien Cristau <jcristau@debian.org> wrote:

On Sat, Mar  3, 2012 at 09:55:51 -0600, Patrick Baggett wrote:

> Where can I read the source for "nbd-tester-client.c"? I just did a quick
> scan of "ghash.c" but I didn't see anything really silly going on, so I'd
> like to check this file. All copies I can find of "nbd-tester-client.c"
> seem to be short (<600 lines) and not use glib at all.
>

It does this:

/*
 * This is the reply packet that nbd-server sends back to the client after
 * it has completed an I/O request (or an error occurs).
 */
struct nbd_reply {
       __be32 magic;
       __be32 error;           /* 0 = ok, else error   */
       char handle[8];         /* handle you got from request  */
};
[...]

GHashTable *handlehash = g_hash_table_new(g_int64_hash, g_int64_equal);

struct nbd_reply rep;

READ_ALL_ERRCHK(sock,
               &rep,
               sizeof(struct nbd_reply),
               err_open,
               "Could not read from server socket: %s",
               strerror(errno));
[...]
prc = g_hash_table_lookup(handlehash, rep.handle);

So alignof(struct nbd_reply) is 4, and rep.handle is not always 8-byte
aligned.  Either the handle should be made a uint64_t, or the test
should do

uint64_t handle;
memcpy(&handle, &rep.handle, 8);
prc = g_hash_table_lookup(handlehash, &handle);

Cheers,
Julien