Patchwork Fix PR gcov-profile/54487 (profiledbootstrap intermittent failures) (issue6496113)

login
register
mail settings
Submitter Teresa Johnson
Date Sept. 12, 2012, 8:45 p.m.
Message ID <20120912204512.7ABC861490@tjsboxrox.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/183432/
State New
Headers show

Comments

Teresa Johnson - Sept. 12, 2012, 8:45 p.m.
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.


--
This patch is available for review at http://codereview.appspot.com/6496113
Jakub Jelinek - Sept. 12, 2012, 8:54 p.m.
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
Teresa Johnson - Sept. 12, 2012, 9:12 p.m.
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

Patch

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"