Issue 6496113: [PATCH] Fix PR gcov-profile/54487 (profiledbootstrap intermittent failures)
|
Created:
12 years, 7 months ago by tejohnson Modified:
10 years, 7 months ago Reviewers:
jakub CC:
gcc-patches_gcc.gnu.org Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/trunk/gcc/ Visibility:
Public. |
Patch Set 1 #MessagesTotal messages: 5
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> * configure.ac(HOST_HAS_F_SETLKW): Set based on compile test using F_SETLKW with fcntl. * configure, config.in: Regenerate. 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" -- This patch is available for review at http://codereview.appspot.com/6496113
Sign in to reply to this message.
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
Sign in to reply to this message.
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 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
On Wed, Sep 12, 2012 at 2:12 PM, Teresa Johnson <tejohnson@google.com> wrote: > 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. Patch committed. I have the 4_7 backport ready. It is basically the same patch. Also tested with bootstap and profiledbootstrap. Ok for gcc/4_7? Thanks, Teresa 2012-09-12 Teresa Johnson <tejohnson@google.com> Backport from mainline. 2012-09-12 Teresa Johnson <tejohnson@google.com> PR gcov-profile/54487 * configure.ac (HOST_HAS_F_SETLKW): Set based on compile test using F_SETLKW with fcntl. * configure, config.in: Regenerate. Index: configure =================================================================== --- configure (revision 191237) +++ configure (working copy) @@ -10968,6 +10968,46 @@ $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" @@ -17970,7 +18010,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 17973 "configure" +#line 18013 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18076,7 +18116,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18079 "configure" +#line 18119 "configure" #include "confdefs.h" #if HAVE_DLFCN_H Index: config.in =================================================================== --- config.in (revision 191237) +++ config.in (working copy) @@ -1588,6 +1588,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 191237) +++ configure.ac (working copy) @@ -1187,6 +1187,22 @@ 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" > > Teresa > >> >> Jakub > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
On Wed, Sep 12, 2012 at 10:06:02PM -0700, Teresa Johnson wrote: > Patch committed. I have the 4_7 backport ready. It is basically the > same patch. Also tested with bootstap and profiledbootstrap. Ok for > gcc/4_7? Yes, thanks. > 2012-09-12 Teresa Johnson <tejohnson@google.com> > > Backport from mainline. > 2012-09-12 Teresa Johnson <tejohnson@google.com> > > PR gcov-profile/54487 > * configure.ac (HOST_HAS_F_SETLKW): Set based on compile > test using F_SETLKW with fcntl. > * configure, config.in: Regenerate. Jakub
Sign in to reply to this message.
|
