7 messages in org.freebsd.freebsd-currentRe: segfault in vfscanf(3): clang and...| From | Sent On | Attachments |
|---|---|---|
| Jean-Sébastien Pédron | Apr 24, 2012 12:49 pm |  .c, .makefile |
| Dimitry Andric | Apr 25, 2012 11:57 am | |
| Boris Samorodov | Apr 25, 2012 12:12 pm | |
| Dimitry Andric | Apr 25, 2012 12:40 pm | |
| Jean-Sébastien Pédron | Apr 26, 2012 2:40 am | .patch |
| Boris Samorodov | Apr 26, 2012 3:00 pm | |
| Jean-Sébastien Pédron | Apr 30, 2012 4:34 am |
| Subject: | Re: segfault in vfscanf(3): clang and __restrict usage | |
|---|---|---|
| From: | Dimitry Andric (di...@FreeBSD.org) | |
| Date: | Apr 25, 2012 11:57:26 am | |
| List: | org.freebsd.freebsd-current | |
On 2012-04-24 21:49, Jean-Sébastien Pédron wrote:
Hi everyone,
vfscanf(3) in HEAD (r234606) segfaults when compiled with clang. For instance, here is a call made in cmake which crashes: fscanf(f, "%*[^\n]\n");
Using r234549 here, everything compiled with clang, but I cannot make that statement crash, whatever I do. Do you have a specific input file which crashes it?
The same libc, compiled with GCC, doesn't segfault.
When it encounters a character class, __svfscanf() calls convert_ccl():
static const int suppress; #define SUPPRESS_PTR ((void *)&suppress)
static __inline int convert_ccl(FILE *fp, char * __restrict p, [...]) { [...]
if (p == SUPPRESS_PTR) { [...] } else { [...] }
[...] } ... From what I understand about __restrict, it indicates that the pointer is the only one pointing to a resource. In vfscanf.c, "suppress" may be pointed by several pointers at a time, so I think __restrict here is incorrect. But I'm really not sure I got it right. And I don't know either if clang behavior is expected.
Indeed, my first impression was the same, that the use of 'restrict' is wrong here. Namely, if you tell the compiler that 'p' is the *only* pointer pointing to a specific object, and you compare it with any other pointer, the comparison will be unequal by definition.
However, after asking around a bit, it seems that clang is still wrong in this particular case. Although this is probably language lawyer area, so beware. :)
I have filed an LLVM PR for it here:
http://llvm.org/bugs/show_bug.cgi?id=12656
Meanwhile, I really wonder why the __restrict keyword was used in this implementation. There are lots of cases in vfscanf.c, where a pointer is declared __restrict, and then aliasing seems to be done anyway.
Besides, I'm not really sure about the potential optimization gains of adding the keyword. With our base gcc, removing all the __restrict keywords results in no binary change. With gcc 4.7, there are some very minor changes, but they are extremely unlikely to gain any performance.
And with clang, there are quite some differences, but apparently it optimizes too aggressively, so more testing is required to see if the potential for bugs outweighs the performance gains.
_______________________________________________ free...@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "free...@freebsd.org"