Re: segfault in vfscanf(3): clang and __restrict usage
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 27.04.2012 00:01, Boris Samorodov wrote: I've rebuild the world (because I had to use gcc-built world for obvious reason) and now smartd works (can't test cupsd for now). BTW, the port devel/ORBit2 which segfaulted both with clang and gcc is built fine now. Thanks! I committed the patch to HEAD r234836. Thank you both for your feedback and sorry for the delay. - -- Jean-Sébastien Pédron -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+eePEACgkQa+xGJsFYOlMD5QCcDh/qLnHOysRjWjsR9o18FxHv oTkAnRoAXi4t3QCDk7tiQeVM7FDqB07c =1SPA -END PGP SIGNATURE- ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: segfault in vfscanf(3): clang and __restrict usage
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 25.04.2012 21:40, Dimitry Andric wrote:
I think the easiest solution for now is to #undef __restrict at the
top of both lib/libc/stdio/vfscanf.c and lib/libc/stdio/vfwscanf.c,
then recompile and reinstall libc.
I attached a patch that removes the __restrict keyword in the
convert_* functions because I believe it's incorrect.
In vfscanf.c, I kept the last one in parseint() because I believe it's
correct: the restricted pointer is used to initialized another one and
comparisons are made only between those two pointers. But I didn't
check if clang optimized out the comparisons. What do you think about
the correctness here?
We can't remove it in vfwscanf(), vfwscanf_l() and __vfwscanf()
(vfwscanf.c) because it's required by the vfwscanf(3) API.
The patch also removes some trailing whitespaces while here.
I'm rebuilding world with this patch and will check that cmake is
working again (the program which shows segfaults for me).
Boris, could you please test it and tell me if cupsd works again for
you too? You just need to rebuild/reinstall the libc, not cups.
Dimitry, thank you for the reporting the problem to LLVM!
- --
Jean-Sébastien Pédron
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk+ZGDoACgkQa+xGJsFYOlOA4ACdH7vH4XfjH6nxcV/axmAYKUKq
8hoAoMrfly9RkUL0UKSKsmbxIiBz2YZy
=GFTc
-END PGP SIGNATURE-
diff --git a/lib/libc/stdio/vfscanf.c b/lib/libc/stdio/vfscanf.c
index 6a6b19c..5316a7c 100644
--- a/lib/libc/stdio/vfscanf.c
+++ b/lib/libc/stdio/vfscanf.c
@@ -125,7 +125,7 @@ static const mbstate_t initial_mbs;
*/
static __inline int
-convert_char(FILE *fp, char * __restrict p, int width)
+convert_char(FILE *fp, char * p, int width)
{
int n, nread;
@@ -152,7 +152,7 @@ convert_char(FILE *fp, char * __restrict p, int width)
nread += sum;
} else {
size_t r = __fread(p, 1, width, fp);
-
+
if (r == 0)
return (-1);
nread += r;
@@ -205,7 +205,7 @@ convert_wchar(FILE *fp, wchar_t *wcp, int width)
}
static __inline int
-convert_ccl(FILE *fp, char * __restrict p, int width, const char *ccltab)
+convert_ccl(FILE *fp, char * p, int width, const char *ccltab)
{
char *p0;
int n;
@@ -304,7 +304,7 @@ convert_wccl(FILE *fp, wchar_t *wcp, int width, const char
*ccltab)
}
static __inline int
-convert_string(FILE *fp, char * __restrict p, int width)
+convert_string(FILE *fp, char * p, int width)
{
char *p0;
int n;
@@ -467,7 +467,7 @@ parseint(FILE *fp, char * __restrict buf, int width, int
base, int flags)
goto ok;
}
break;
-
+
/*
* x ok iff flag still set 2nd char (or 3rd char if
* we have a sign).
diff --git a/lib/libc/stdio/vfwscanf.c b/lib/libc/stdio/vfwscanf.c
index 6b4d8c5..9d628b0 100644
--- a/lib/libc/stdio/vfwscanf.c
+++ b/lib/libc/stdio/vfwscanf.c
@@ -138,7 +138,7 @@ static const mbstate_t initial_mbs;
*/
static __inline int
-convert_char(FILE *fp, char * __restrict mbp, int width, locale_t locale)
+convert_char(FILE *fp, char * mbp, int width, locale_t locale)
{
mbstate_t mbs;
size_t nconv;
@@ -192,7 +192,7 @@ convert_wchar(FILE *fp, wchar_t *wcp, int width, locale_t
locale)
}
static __inline int
-convert_ccl(FILE *fp, char * __restrict mbp, int width, const struct ccl *ccl,
+convert_ccl(FILE *fp, char * mbp, int width, const struct ccl *ccl,
locale_t locale)
{
mbstate_t mbs;
@@ -261,7 +261,7 @@ convert_wccl(FILE *fp, wchar_t *wcp, int width, const
struct ccl *ccl,
}
static __inline int
-convert_string(FILE *fp, char * __restrict mbp, int width, locale_t locale)
+convert_string(FILE *fp, char * mbp, int width, locale_t locale)
{
mbstate_t mbs;
size_t nconv;
@@ -407,7 +407,7 @@ parseint(FILE *fp, wchar_t *buf, int width, int base, int
flags,
goto ok;
}
break;
-
+
/*
* x ok iff flag still set 2nd char (or 3rd char if
* we have a sign).
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: segfault in vfscanf(3): clang and __restrict usage
26.04.2012 13:41, Jean-Sébastien Pédron пишет: Boris, could you please test it and tell me if cupsd works again for you too? You just need to rebuild/reinstall the libc, not cups. I've rebuild the world (because I had to use gcc-built world for obvious reason) and now smartd works (can't test cupsd for now). BTW, the port devel/ORBit2 which segfaulted both with clang and gcc is built fine now. Thanks! -- WBR, Boris Samorodov (bsam) FreeBSD Committer, http://www.FreeBSD.org The Power To Serve ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: segfault in vfscanf(3): clang and __restrict usage
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.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: segfault in vfscanf(3): clang and __restrict usage
25.04.2012 22:57, Dimitry Andric пишет: 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? - % uname -a FreeBSD bsam.wart.ru 10.0-CURRENT FreeBSD 10.0-CURRENT #0 r234635: Tue Apr 24 11:41:32 SAMT 2012 b...@bsam.wart.ru:/usr/obj/usr/src/sys/BBX i386 % sudo gdb smartd smartd.core GNU gdb 6.1.1 [FreeBSD] [...] #0 0x33ebdc2e in vfscanf () from /lib/libc.so.7 (gdb) - I think that cupsd also suffer from the bug. BTW, I have the system and almost all ports compiled (tomorrow and today) with clang. -- WBR, Boris Samorodov (bsam) FreeBSD Committer, http://www.FreeBSD.org The Power To Serve ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: segfault in vfscanf(3): clang and __restrict usage
On 2012-04-25 21:13, Boris Samorodov wrote: 25.04.2012 22:57, Dimitry Andric пишет: 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? - % uname -a FreeBSD bsam.wart.ru 10.0-CURRENT FreeBSD 10.0-CURRENT #0 r234635: Tue Apr 24 11:41:32 SAMT 2012 b...@bsam.wart.ru:/usr/obj/usr/src/sys/BBX i386 % sudo gdb smartd smartd.core GNU gdb 6.1.1 [FreeBSD] [...] #0 0x33ebdc2e in vfscanf () from /lib/libc.so.7 (gdb) - I think that cupsd also suffer from the bug. BTW, I have the system and almost all ports compiled (tomorrow and today) with clang. Looks like the __restricted keywords were introduced just two days ago, in r234585, which may be why I didn't see any crashes yet. I think the easiest solution for now is to #undef __restrict at the top of both lib/libc/stdio/vfscanf.c and lib/libc/stdio/vfwscanf.c, then recompile and reinstall libc. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
segfault in vfscanf(3): clang and __restrict usage
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
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);
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 {
[...]
}
[...]
}
In this case, there's no argument following the format string, and
convert_ccl is called with p = SUPPRESS_PTR. Therefore, we should
enter the if{} block. But when compiled with clang, we enter the
else{} block (causing the segfault).
I made a small program that shows the problem (attached): it seems to
be related to the __restrict qualifier.
Compiled with GCC:
./ptr-comp
p=0x600ac8 vs. SUPPRESS_PTR=0x600ac8
p == SUPPRESS_PTR
Compiled with clang:
./ptr-comp
p=0x4007dc vs. SUPPRESS_PTR=0x4007dc
p != SUPPRESS_PTR - WRONG
- 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.
What do you think?
- --
Jean-Sébastien Pédron
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk+XA8kACgkQa+xGJsFYOlOt9wCffUwQ344hfanDzU27wdgW5C+t
4fYAoKPh26OW/ge+VbLaOMTT/YtUYOwM
=OblW
-END PGP SIGNATURE-
#include stdio.h
static const int suppress;
#define SUPPRESS_PTR((void *)suppress)
void
func(char * __restrict p)
{
printf(p=%p vs. SUPPRESS_PTR=%p\n, p, SUPPRESS_PTR);
if (p == SUPPRESS_PTR)
printf(p == SUPPRESS_PTR\n);
else
printf(p != SUPPRESS_PTR - WRONG\n);
}
int
main(int argc, char *argv [])
{
char *p;
p = SUPPRESS_PTR;
func(p);
return (0);
}
PROG = ptr-comp
.include bsd.prog.mk
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
