-fcatch-undefined-behavior false positive with readdir()?

Hi all,

The following C code works without -fcatch-undefined-behavior, and worked with it too, until a few weeks ago when it was given new smarts:

Just a hunch: is d->d_name an unaligned pointer? We ran into a similar
issue with gethostbyname.

-- Joe Ranieri

Joe,

Thanks for your reply. But I'm afraid I don't follow. Unaligned with respect to what? The 'struct dirent' is declared by the system basically like so:

#pragma pack()
struct dirent {
  uint64_t d_ino;
  uint64_t d_seekoff;
  uint16_t d_reclen;
  uint16_t d_namlen;
  uint8_t d_type;
  char d_name[1024];
}

Note the 'pragma pack'; is that the alignment you're referring to? On my system, I don't see any pragma pack with gethostbyname()'s 'struct hostent', what problem did you have with it?

I just tried something else: it's not just the d_name field, but accessing any field triggers the SIGILL.

Cheers,

Can you try adding

printf ("%p %zd\n", d, _Alignof(*d));

before the line which fails? Is the pointer correctly aligned?

Richard,

It prints 8, for every iteration of the loop. I guess that's correct since the first field of the struct is a uint64_t.

Sean

Richard,

It prints 8, for every iteration of the loop. I guess that’s correct since the first field of the struct is a uint64_t.

What pointer values does it print? Are they 8 byte aligned?

Aha! So I changed your printf() to:

  printf ("%p %zd r=%lu\n", d, _Alignof(*d), (uintptr_t)d % 8);

and now:

0x7fb758800000 8 r=0
0x7fb758800018 8 r=0
0x7fb758800030 8 r=0
0x7fb75880004c 8 r=4
0x7fb758800070 8 r=0
0x7fb758800094 8 r=4

Indeed the first non-aligned one crashes. If I add an 'if' and skip all the non 8 byte aligned ones, -fcatch-undefined-behavior no longer complains.

So this is an OS X bug then? (I'm pretty sure my code snippet itself is correct.)

Cheers,

Yes, it appears so. (Not sure whether it's a bug in the header or a
bug in the implementation.)

-Eli

Ugh. I was in 10.7.5, just tried in 10.8.2 and seems someone has already fixed this.

Anyway, I think it's a nice example of why fine-graned control over -fcatch-undefined-behavior would be nice: enabling it can find bugs that you can't fix. If I could turn off the unaligned memory check, at least I could proceed and maybe find some bugs in my own code...

Thanks to all for helping.

Cheers,

Note that you can work around this particular bug by using readdir_r()
with a dirent you allocate correctly.

If you look at some of Richard's proposed changes to the undefined
behavior checking, they include caret diagnostics (just like you're
used to getting from clang). In this case (correct me if I'm wrong,
Richard; I'm working from memory of the other thread), you'd get a
nice, colorized output of something like:

$ ./a.out
Undefined behavior! Unaligned load from pointer 'd' (address
0x7fb75880004c) of type 'struct dirent' that requires alignment 8:
const char* local = d->d_name; // bam!
                           ^~~

--Sean Silva

$ ./a.out
dirent.cpp:4:19: fatal error: member access within misaligned address 0x7ffff752f787 for type ‘struct dirent’ requiring 8 byte alignment

zsh: illegal hardware instruction (core dumped) ./a.out

$

… coming soon to a -fcatch-undefined-behavior near you. Snippet, backtrace, etc. to follow.

Please don't do that. Allocating the buffer is very difficult and impossible on some platforms. Sticking to readdir is much more reasonable.

how is this reporting emitted exactly? in particular, will it be usable
from a kernel context? i've been trying to enable -fcatch-undefined-behavior
for linux for some time now and it caught interesting bugs but with the
addition of alignment checking it doesn't even get past self-decompression ;).
obviously any reporting at that point is not exactly trivial and were it
not for qemu, it'd be painful to debug... so what i'm trying to say is
that verbose reporting is nice but a mechanism to selectively disable UB
checking subfeatures would also be useful.

cheers,
  PaX Team

how is this reporting emitted exactly? in particular, will it be usable
from a kernel context? i've been trying to enable -fcatch-undefined-behavior
for linux for some time now and it caught interesting bugs but with the
addition of alignment checking it doesn't even get past self-decompression ;).
obviously any reporting at that point is not exactly trivial and were it
not for qemu, it'd be painful to debug...

The instrumentation would need to be adjusted, but I don't think it
will be that difficult to get a reasonable behavior. You can just
core-dump the kernel, even if you're running on bare metal. You of
course will have to protect from a "recursive" UB dump in the IO path
when writing out the core, but otherwise it's pretty straightforward.

Another option which is basically the same but would allow for the
kernel to continue operating uninterrupted would be to essentially
"fork" the kernel and leave it frozen there in the state where the UB
triggered. You could then take advantage of Copy-On-Write to avoid the
lengthy dumping procedure, while still preserving the state of the
kernel when the UB occurred. This wouldn't require too much code, but
the code that it does need is very "tricky".

--Sean Silva

I'm afraid I misspoke! I was using an older clang on 10.8. The bug is present in 10.7.5 and 10.8.2, and I've filed:

<rdar://problem/12388003>

Cheers,