You are viewing deviant_

THE MAGIC WORDS ARE SQUEAMISH OSSIFRAGE - This is not 'Nam. This is C. There are rules. [entries|archive|friends|userinfo]
THE MAGIC WORDS ARE SQUEAMISH OSSIFRAGE

[ website | My Website ]
[ userinfo | livejournal userinfo ]
[ archive | journal archive ]

This is not 'Nam. This is C. There are rules. [Nov. 29th, 2006|04:42 pm]
Previous Entry Share Next Entry
I was going about my day earlier, when spot sent me an IM asking for help with one of my favorite programs. Apparently, it was failing quite spectacularly on sparc, and he was having some trouble making the machine give useful debug data.

After a bit of hacking on it, I found this wonderful nugget:
int label_read(struct device *dev, struct label **result)
{
        char buf[LABEL_SIZE];
        struct labeller *l;
...
        if (!(l = _find_labeller(dev, buf, §or)))
                goto_out;       
...
}
...
static struct labeller *_find_labeller(struct device *dev, char *buf,
                                       uint64_t *label_sector)
{
...
        struct label_header *lh;
...
        uint64_t sector;
        int found = 0;
        char readbuf[LABEL_SCAN_SIZE];

        if (!dev_read(dev, UINT64_C(0), LABEL_SCAN_SIZE, readbuf)) {
                log_debug("%s: Failed to read label area", dev_name(dev));
                goto out;
        }

        /* Scan first few sectors for a valid label */
        for (sector = 0; sector < LABEL_SCAN_SECTORS;
             sector += LABEL_SIZE >> SECTOR_SHIFT) {
                lh = (struct label_header *) (readbuf +
                                              (sector << SECTOR_SHIFT));
...
                list_iterate_items(li, &_labellers) {
                        if (li->l->ops->can_handle(li->l, (char *) lh, sector)) {
...
}
Where li->l->ops->can_handle() winds up being...
static int _pool_read(struct labeller *l, struct device *dev, char *buf,
                 struct label **label)
{
        struct pool_list pl;

        return read_pool_label(&pl, l, dev, buf, label);
}
...
int read_pool_label(struct pool_list *pl, struct labeller *l,
                    struct device *dev, char *buf, struct label **label)
{               
...
        struct pool_disk *pd = &pl->pd;

        pool_label_in(pd, buf);
...
}
And elsewhere we find:
struct pool_disk {
        uint64_t pl_magic;      /* Pool magic number */
        uint64_t pl_pool_id;    /* Unique pool identifier */
        char pl_pool_name[POOL_NAME_SIZE];      /* Name of pool */
...
}
...
#define CPIN_64(x, y) {(x) = xlate64_be((y));}
...
void pool_label_in(struct pool_disk *pl, char *buf)
{
        struct pool_disk *bufpl = (struct pool_disk *) buf;

        CPIN_64(pl->pl_magic, bufpl->pl_magic);
        CPIN_64(pl->pl_pool_id, bufpl->pl_pool_id);
        CPIN_8(pl->pl_pool_name, bufpl->pl_pool_name, POOL_NAME_SIZE);
        CPIN_32(pl->pl_version, bufpl->pl_version);
...
}
Who wants to play "hunt the SIGBUS"?

Just think. Somebody was paid real money to write this. The mind boggles.
linkReply

Comments:
[User Picture]From: ndykman
2006-11-29 10:19 pm (UTC)

(Link)

Wow. I've been in my PhD program too long. None of this makes sense. Of course, this

&pl->pd

makes me itch. Parens are your friends!

[User Picture]From: deviant_
2006-11-29 11:08 pm (UTC)

(Link)

Nah, that part's fine.

The real problem here is that what you wind up doing is
char buf[1024];
struct lose {
 uint64_t x;
} *s;
uint64_t x;

s = (struct lose *)buf[1];
x = *s;
About which the C Standard sez:
A pointer to an object or incomplete type may be converted to a pointer to a different object or incomplete type. If the resulting pointer is not correctly aligned for the pointed-to type, the behavior is undefined.
So really an implementation can do whatever it wants here. Most implementations just let it work. On SPARC you get a SIGBUS and your program dumps core and goes away.
[User Picture]From: ndykman
2006-11-30 02:34 am (UTC)

(Link)

Check. I am in awe of your C foo. Can gcc warn about this if you crank up the warning levels?
[User Picture]From: deviant_
2006-11-30 07:24 am (UTC)

(Link)

Not really -- it only happens when you use type-casting, which is really telling the compiler "I know what I'm doing, don't worry about this part".

The irony is wonderful. Too bad about the result.
From: gorski
2006-11-30 01:26 pm (UTC)

(Link)

Of course, that's the thing about undefined behavior--you never really know.

It could just set cheshire's socks on fire.

WELCOME TO HUNT THE SIGBUS.

THE SIGBUS LIVES IN A PROGRAM OF TWENTY FUNCTIONS. EACH FUNCTION HAS THREE CALLS LEADING TO OTHER FUNCTIONS. (LOOK AT A DODECAHEDRON TO SEE HOW THIS WORKS. IF YOU DON'T KNOW WHAT A DODECAHEDRON IS, ASK SOMEONE.)

HAZARDS:

POINTER TO FISHKILL: IF YOU FOLLOW A POINTER TO A NONEXISTANT PHYSICAL ADDRESS, YOU FIND THE SIGBUS (& LOSE)!

BEEFDEAD: IF YOU FOLLOW A BADLY ALIGNED POINTER, YOU ALSO FIND THE SIGBUS (& LOSE)!



...peace to you,

--me
[User Picture]From: base10
2006-12-03 01:34 am (UTC)

(Link)

YOU FEEL A MEMORY LEAK FROM THE NEXT ROOM.
[User Picture]From: fezzgig
2006-12-01 04:32 am (UTC)

(Link)

Best subject-line evar.

Evar.