| Message ID | 20120912204512.7ABC861490@tjsboxrox.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 12, 2012 at 01:45:12PM -0700, Teresa Johnson wrote: > This fixes PR gcov-profile/54487 where the gcda files were not locked > by the profile-use read, enabling writes by other instrumented compiles > to change the profile in the middle of the profile use read. The GCOV_LOCKED > macro was not set because it was guarded by HOST_HAS_F_SETLKW, which was > never set. The fix is to add a compile test in the configure to set it. > > Tested with bootstrap and profiledbootstrap on x86_64-unknown-linux-gnu. > Ok for trunk? > > Thanks, > Teresa > > 2012-09-12 Teresa Johnson <tejohnson@google.com> > Please include PR gcov-profile/54487 here in the ChangeLog entry. > * configure.ac(HOST_HAS_F_SETLKW): Set based on compile Space before (. > test using F_SETLKW with fcntl. > * configure, config.in: Regenerate. > > --- configure.ac (revision 191225) > +++ configure.ac (working copy) > @@ -1159,6 +1159,16 @@ if test $gcc_cv_type_clock_t = yes; then > [Define if <time.h> defines clock_t.]) > fi > > +# Check if F_SETLKW is supported by fcntl. > +AC_CACHE_CHECK(for F_SETLKW, ac_cv_f_setlkw, [ > +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ > +#include "fcntl.h" Please use #include <fcntl.h> instead. > +]], [[struct flock fl; fl.l_whence = 0; fl.l_start = 0; fl.l_len = 0; fl.l_pid = 0; return fcntl (1, F_SETLKW, &fl);]])],[ac_cv_f_setlkw=yes],[ac_cv_f_setlkw=no])]) And split this overlong line, there is no reason why you can't use a newline e.g. after every ; in the test proglet. > +if test $ac_cv_f_setlkw = yes; then > + AC_DEFINE(HOST_HAS_F_SETLKW, 1, > + [Define if F_SETLKW supported by fcntl.]) > +fi > + > # Restore CFLAGS, CXXFLAGS from before the gcc_AC_NEED_DECLARATIONS tests. > CFLAGS="$saved_CFLAGS" > CXXFLAGS="$saved_CXXFLAGS" Ok for trunk with those changes, IMHO it would be worthwhile to put this into 4.7 too, I've seen several unexplained profiledbootstrap errors on that branch in the past already when using make -jN with high N. Jakub
On Wed, Sep 12, 2012 at 1:54 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Sep 12, 2012 at 01:45:12PM -0700, Teresa Johnson wrote: >> This fixes PR gcov-profile/54487 where the gcda files were not locked >> by the profile-use read, enabling writes by other instrumented compiles >> to change the profile in the middle of the profile use read. The GCOV_LOCKED >> macro was not set because it was guarded by HOST_HAS_F_SETLKW, which was >> never set. The fix is to add a compile test in the configure to set it. >> >> Tested with bootstrap and profiledbootstrap on x86_64-unknown-linux-gnu. >> Ok for trunk? >> >> Thanks, >> Teresa >> >> 2012-09-12 Teresa Johnson <tejohnson@google.com> >> > > Please include > PR gcov-profile/54487 > here in the ChangeLog entry. > >> * configure.ac(HOST_HAS_F_SETLKW): Set based on compile > > Space before (. > >> test using F_SETLKW with fcntl. >> * configure, config.in: Regenerate. >> >> --- configure.ac (revision 191225) >> +++ configure.ac (working copy) >> @@ -1159,6 +1159,16 @@ if test $gcc_cv_type_clock_t = yes; then >> [Define if <time.h> defines clock_t.]) >> fi >> >> +# Check if F_SETLKW is supported by fcntl. >> +AC_CACHE_CHECK(for F_SETLKW, ac_cv_f_setlkw, [ >> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ >> +#include "fcntl.h" > > Please use > #include <fcntl.h> > instead. > >> +]], [[struct flock fl; fl.l_whence = 0; fl.l_start = 0; fl.l_len = 0; fl.l_pid = 0; return fcntl (1, F_SETLKW, &fl);]])],[ac_cv_f_setlkw=yes],[ac_cv_f_setlkw=no])]) > > And split this overlong line, there is no reason why you can't use a newline > e.g. after every ; in the test proglet. > >> +if test $ac_cv_f_setlkw = yes; then >> + AC_DEFINE(HOST_HAS_F_SETLKW, 1, >> + [Define if F_SETLKW supported by fcntl.]) >> +fi >> + >> # Restore CFLAGS, CXXFLAGS from before the gcc_AC_NEED_DECLARATIONS tests. >> CFLAGS="$saved_CFLAGS" >> CXXFLAGS="$saved_CXXFLAGS" > > Ok for trunk with those changes, IMHO it would be worthwhile to put this > into 4.7 too, I've seen several unexplained profiledbootstrap errors on > that branch in the past already when using make -jN with high N. Ok, thanks. I will fix the issues you pointed out above and retest before committing. I'll prepare a backport patch for 4.7 as well. Teresa > > Jakub
Index: configure =================================================================== --- configure (revision 191225) +++ configure (working copy) @@ -10731,6 +10731,41 @@ $as_echo "#define HAVE_CLOCK_T 1" >>confdefs.h fi +# Check if F_SETLKW is supported by fcntl. +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for F_SETLKW" >&5 +$as_echo_n "checking for F_SETLKW... " >&6; } +if test "${ac_cv_f_setlkw+set}" = set; then : + $as_echo_n "(cached) " >&6 +else + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +#include "fcntl.h" + +int +main () +{ +struct flock fl; fl.l_whence = 0; fl.l_start = 0; fl.l_len = 0; fl.l_pid = 0; return fcntl (1, F_SETLKW, &fl); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + ac_cv_f_setlkw=yes +else + ac_cv_f_setlkw=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_f_setlkw" >&5 +$as_echo "$ac_cv_f_setlkw" >&6; } +if test $ac_cv_f_setlkw = yes; then + +$as_echo "#define HOST_HAS_F_SETLKW 1" >>confdefs.h + +fi + # Restore CFLAGS, CXXFLAGS from before the gcc_AC_NEED_DECLARATIONS tests. CFLAGS="$saved_CFLAGS" CXXFLAGS="$saved_CXXFLAGS" @@ -17742,7 +17777,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 17745 "configure" +#line 17780 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -17848,7 +17883,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 17851 "configure" +#line 17886 "configure" #include "confdefs.h" #if HAVE_DLFCN_H Index: config.in =================================================================== --- config.in (revision 191225) +++ config.in (working copy) @@ -1600,6 +1600,12 @@ #endif +/* Define if F_SETLKW supported by fcntl. */ +#ifndef USED_FOR_TARGET +#undef HOST_HAS_F_SETLKW +#endif + + /* Define as const if the declaration of iconv() needs const. */ #ifndef USED_FOR_TARGET #undef ICONV_CONST Index: configure.ac =================================================================== --- configure.ac (revision 191225) +++ configure.ac (working copy) @@ -1159,6 +1159,16 @@ if test $gcc_cv_type_clock_t = yes; then [Define if <time.h> defines clock_t.]) fi +# Check if F_SETLKW is supported by fcntl. +AC_CACHE_CHECK(for F_SETLKW, ac_cv_f_setlkw, [ +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#include "fcntl.h" +]], [[struct flock fl; fl.l_whence = 0; fl.l_start = 0; fl.l_len = 0; fl.l_pid = 0; return fcntl (1, F_SETLKW, &fl);]])],[ac_cv_f_setlkw=yes],[ac_cv_f_setlkw=no])]) +if test $ac_cv_f_setlkw = yes; then + AC_DEFINE(HOST_HAS_F_SETLKW, 1, + [Define if F_SETLKW supported by fcntl.]) +fi + # Restore CFLAGS, CXXFLAGS from before the gcc_AC_NEED_DECLARATIONS tests. CFLAGS="$saved_CFLAGS" CXXFLAGS="$saved_CXXFLAGS"