public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, libiberty] Fix build regression
@ 2010-11-19 23:00 Anthony Green
  2010-11-20 10:53 ` Ralf Wildenhues
  2010-11-20 15:16 ` Andi Kleen
  0 siblings, 2 replies; 6+ messages in thread
From: Anthony Green @ 2010-11-19 23:00 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

This patch fixes a recent libiberty build regression discussed in this
thread:
  http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01998.html

If we know we aren't going to be able to build executables, simply emit
a warning saying that we can't link (instead of exiting with an error),
and mention that we're assuming prctl does not exist for this target.

Ok to commit?

AG



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libiberty-link-patch.txt --]
[-- Type: text/x-c; name="libiberty-link-patch.txt";, Size: 2268 bytes --]

2010-11-19  Anthony Green  <green@moxielogic.com>

	* configure.ac: Don't run link tests if we can't build
	executables.
	* configure: Rebuilt.


Index: libiberty/configure.ac
===================================================================
--- libiberty/configure.ac	(revision 166958)
+++ libiberty/configure.ac	(working copy)
@@ -538,15 +538,20 @@
 AC_SUBST(CHECK)
 AC_SUBST(target_header_dir)
 
-# check for prctl PR_SET_NAME
-AC_LINK_IFELSE([AC_LANG_SOURCE([[
-#include <sys/prctl.h>
-int main()
-{
-  return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
-}
-]])], AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
-	[Define if you have prctl PR_SET_NAME]))
+if test x$gcc_no_link = xyes; then
+  AC_MSG_WARN([
+*** Cannot link executables.  Assuming target does not have prctl function.])
+else
+  # check for prctl PR_SET_NAME
+  AC_LINK_IFELSE([AC_LANG_SOURCE([[
+  #include <sys/prctl.h>
+  int main()
+  {
+    return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
+  }
+  ]])], AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
+  	[Define if you have prctl PR_SET_NAME]))
+fi
 
 case "${host}" in
   *-*-cygwin* | *-*-mingw*)
Index: libiberty/configure
===================================================================
--- libiberty/configure	(revision 166958)
+++ libiberty/configure	(working copy)
@@ -5719,18 +5719,24 @@
 
 
 
-# check for prctl PR_SET_NAME
 if test x$gcc_no_link = xyes; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
+*** Cannot link executables.  Assuming target does not have prctl function." >&5
+$as_echo "$as_me: WARNING:
+*** Cannot link executables.  Assuming target does not have prctl function." >&2;}
+else
+  # check for prctl PR_SET_NAME
+  if test x$gcc_no_link = xyes; then
   as_fn_error "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5
 fi
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
-#include <sys/prctl.h>
-int main()
-{
-  return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
-}
+  #include <sys/prctl.h>
+  int main()
+  {
+    return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
+  }
 
 _ACEOF
 if ac_fn_c_try_link "$LINENO"; then :
@@ -5740,6 +5746,7 @@
 fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext conftest.$ac_ext
+fi
 
 case "${host}" in
   *-*-cygwin* | *-*-mingw*)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, libiberty] Fix build regression
  2010-11-19 23:00 [Patch, libiberty] Fix build regression Anthony Green
@ 2010-11-20 10:53 ` Ralf Wildenhues
  2010-11-20 13:29   ` Anthony Green
  2010-11-20 15:16 ` Andi Kleen
  1 sibling, 1 reply; 6+ messages in thread
From: Ralf Wildenhues @ 2010-11-20 10:53 UTC (permalink / raw)
  To: Anthony Green; +Cc: gcc-patches, Andi Kleen

* Anthony Green wrote on Fri, Nov 19, 2010 at 11:09:36PM CET:
> This patch fixes a recent libiberty build regression discussed in this
> thread:
>   http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01998.html
> 
> If we know we aren't going to be able to build executables, simply emit
> a warning saying that we can't link (instead of exiting with an error),
> and mention that we're assuming prctl does not exist for this target.
> 
> Ok to commit?

A better fix is to actually allow the answer to the test to be correct
even in the no-link case.  For that, cache the test result.  It may then
be useful to move the test to a more appropriate place, such as after
the code that sets all the cache variables based on $host in the no-link
case.  That way port maintainers can enter the right answers for this
test there, and if not given users can still configure toplevel with
  .../configure target_configargs=libiberty_cv_prctl_PR_SET_NAME=yes

or so.

Here's how a cached version of the test would look like (untested!),
including a couple more fixlets:

AC_CACHE_CHECK([for prctl PR_SET_NAME],
  [libiberty_cv_prctl_PR_SET_NAME],
  [AC_LINK_IFELSE(
     [AC_LANG_PROGRAM([[#include <sys/prctl.h>]],
        [[return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;]])],
     [libiberty_cv_prctl_PR_SET_NAME=yes],
     [libiberty_cv_prctl_PR_SET_NAME=no])])

if test "$libiberty_cv_prctl_PR_SET_NAME" = yes; then
  AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
            [Define if you have prctl PR_SET_NAME]))
fi

Thanks,
Ralf

> 2010-11-19  Anthony Green  <green@moxielogic.com>
> 
> 	* configure.ac: Don't run link tests if we can't build
> 	executables.
> 	* configure: Rebuilt.

> --- libiberty/configure.ac	(revision 166958)
> +++ libiberty/configure.ac	(working copy)
> @@ -538,15 +538,20 @@
>  AC_SUBST(CHECK)
>  AC_SUBST(target_header_dir)
>  
> -# check for prctl PR_SET_NAME
> -AC_LINK_IFELSE([AC_LANG_SOURCE([[
> -#include <sys/prctl.h>
> -int main()
> -{
> -  return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
> -}
> -]])], AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
> -	[Define if you have prctl PR_SET_NAME]))
> +if test x$gcc_no_link = xyes; then
> +  AC_MSG_WARN([
> +*** Cannot link executables.  Assuming target does not have prctl function.])
> +else
> +  # check for prctl PR_SET_NAME
> +  AC_LINK_IFELSE([AC_LANG_SOURCE([[
> +  #include <sys/prctl.h>
> +  int main()
> +  {
> +    return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
> +  }
> +  ]])], AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
> +  	[Define if you have prctl PR_SET_NAME]))
> +fi
>  
>  case "${host}" in
>    *-*-cygwin* | *-*-mingw*)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, libiberty] Fix build regression
  2010-11-20 10:53 ` Ralf Wildenhues
@ 2010-11-20 13:29   ` Anthony Green
  2010-11-20 13:37     ` Ralf Wildenhues
  0 siblings, 1 reply; 6+ messages in thread
From: Anthony Green @ 2010-11-20 13:29 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

On 11/20/2010 1:21 AM, Ralf Wildenhues wrote:
> * Anthony Green wrote on Fri, Nov 19, 2010 at 11:09:36PM CET:
>> This patch fixes a recent libiberty build regression discussed in this
>> thread:
>>    http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01998.html
>>
>> If we know we aren't going to be able to build executables, simply emit
>> a warning saying that we can't link (instead of exiting with an error),
>> and mention that we're assuming prctl does not exist for this target.
>>
>> Ok to commit?
>
> A better fix is to actually allow the answer to the test to be correct
> even in the no-link case.  For that, cache the test result.  It may then
> be useful to move the test to a more appropriate place, such as after
> the code that sets all the cache variables based on $host in the no-link
> case.  That way port maintainers can enter the right answers for this
> test there, and if not given users can still configure toplevel with
>    .../configure target_configargs=libiberty_cv_prctl_PR_SET_NAME=yes


Thanks Ralf.  However last night on #gcc Ian questioned why this is a 
link test in the first place.  Why not just check headers instead?  I've 
attached a revised patch based on header checking.

Ok to commit?

AG


[-- Attachment #2: libiberty-link-patch-2.txt --]
[-- Type: text/plain, Size: 4770 bytes --]

2010-11-19  Anthony Green  <green@moxielogic.com>

	* configure.ac: Turn PR_SET_NAME link test into a test for
	sys/prctl.h.
	* configure, config.in: Rebuilt.
	* setproctitle.c: Test for HAVE_SYS_PRCTL_H.
	(setproctitle) Test for PR_SET_NAME definition.


Index: libiberty/configure.ac
===================================================================
--- libiberty/configure.ac	(revision 166973)
+++ libiberty/configure.ac	(working copy)
@@ -246,7 +246,7 @@
 # It's OK to check for header files.  Although the compiler may not be
 # able to link anything, it had better be able to at least compile
 # something.
-AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h)
+AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h sys/prctl.h)
 AC_HEADER_SYS_WAIT
 AC_HEADER_TIME
 
@@ -538,16 +538,6 @@
 AC_SUBST(CHECK)
 AC_SUBST(target_header_dir)
 
-# check for prctl PR_SET_NAME
-AC_LINK_IFELSE([AC_LANG_SOURCE([[
-#include <sys/prctl.h>
-int main()
-{
-  return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
-}
-]])], AC_DEFINE(HAVE_PRCTL_SET_NAME, 1,
-	[Define if you have prctl PR_SET_NAME]))
-
 case "${host}" in
   *-*-cygwin* | *-*-mingw*)
     AC_DEFINE(HAVE_SYS_ERRLIST)
Index: libiberty/setproctitle.c
===================================================================
--- libiberty/setproctitle.c	(revision 166973)
+++ libiberty/setproctitle.c	(working copy)
@@ -20,7 +20,7 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-#ifdef HAVE_PRCTL_SET_NAME
+#ifdef HAVE_SYS_PRCTL_H
 #include <sys/prctl.h>
 #endif
 #include "ansidecl.h"
@@ -39,7 +39,7 @@
 void
 setproctitle (const char *name ATTRIBUTE_UNUSED, ...)
 {
-#ifdef HAVE_PRCTL_SET_NAME
+#ifdef PR_SET_NAME
   /* On Linux this sets the top visible "comm", but not necessarily
      the name visible in ps. */
   prctl (PR_SET_NAME, name);
Index: libiberty/config.in
===================================================================
--- libiberty/config.in	(revision 166973)
+++ libiberty/config.in	(working copy)
@@ -169,9 +169,6 @@
 /* Define to 1 if you have the `on_exit' function. */
 #undef HAVE_ON_EXIT
 
-/* Define if you have prctl PR_SET_NAME */
-#undef HAVE_PRCTL_SET_NAME
-
 /* Define to 1 if you have the <process.h> header file. */
 #undef HAVE_PROCESS_H
 
@@ -304,6 +301,9 @@
 /* Define to 1 if you have the <sys/param.h> header file. */
 #undef HAVE_SYS_PARAM_H
 
+/* Define to 1 if you have the <sys/prctl.h> header file. */
+#undef HAVE_SYS_PRCTL_H
+
 /* Define to 1 if you have the <sys/pstat.h> header file. */
 #undef HAVE_SYS_PSTAT_H
 
Index: libiberty/configure
===================================================================
--- libiberty/configure	(revision 166973)
+++ libiberty/configure	(working copy)
@@ -4895,7 +4895,7 @@
 # It's OK to check for header files.  Although the compiler may not be
 # able to link anything, it had better be able to at least compile
 # something.
-for ac_header in sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h
+for ac_header in sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h sys/prctl.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_preproc "$LINENO" "$ac_header" "$as_ac_Header"
@@ -5719,28 +5719,6 @@
 
 
 
-# check for prctl PR_SET_NAME
-if test x$gcc_no_link = xyes; then
-  as_fn_error "Link tests are not allowed after GCC_NO_EXECUTABLES." "$LINENO" 5
-fi
-cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h.  */
-
-#include <sys/prctl.h>
-int main()
-{
-  return (prctl(PR_SET_NAME, "foo") == 0) ? 0 : 1;
-}
-
-_ACEOF
-if ac_fn_c_try_link "$LINENO"; then :
-
-$as_echo "#define HAVE_PRCTL_SET_NAME 1" >>confdefs.h
-
-fi
-rm -f core conftest.err conftest.$ac_objext \
-    conftest$ac_exeext conftest.$ac_ext
-
 case "${host}" in
   *-*-cygwin* | *-*-mingw*)
     $as_echo "#define HAVE_SYS_ERRLIST 1" >>confdefs.h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, libiberty] Fix build regression
  2010-11-20 13:29   ` Anthony Green
@ 2010-11-20 13:37     ` Ralf Wildenhues
  2010-11-21  8:47       ` Anthony Green
  0 siblings, 1 reply; 6+ messages in thread
From: Ralf Wildenhues @ 2010-11-20 13:37 UTC (permalink / raw)
  To: Anthony Green; +Cc: gcc-patches

* Anthony Green wrote on Sat, Nov 20, 2010 at 01:04:59PM CET:
> Thanks Ralf.  However last night on #gcc Ian questioned why this is
> a link test in the first place.  Why not just check headers instead?
> I've attached a revised patch based on header checking.
> 
> Ok to commit?

That looks fairly safe, and the idea seems good to me too.
OK if it works (you can run 'make configure-libiberty' on a system with
and one without the header to find out quickly).

Thanks,
Ralf

> 2010-11-19  Anthony Green  <green@moxielogic.com>
> 
> 	* configure.ac: Turn PR_SET_NAME link test into a test for
> 	sys/prctl.h.
> 	* configure, config.in: Rebuilt.
> 	* setproctitle.c: Test for HAVE_SYS_PRCTL_H.
> 	(setproctitle) Test for PR_SET_NAME definition.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, libiberty] Fix build regression
  2010-11-19 23:00 [Patch, libiberty] Fix build regression Anthony Green
  2010-11-20 10:53 ` Ralf Wildenhues
@ 2010-11-20 15:16 ` Andi Kleen
  1 sibling, 0 replies; 6+ messages in thread
From: Andi Kleen @ 2010-11-20 15:16 UTC (permalink / raw)
  To: Anthony Green; +Cc: gcc-patches, Andi Kleen

> This patch fixes a recent libiberty build regression discussed in this
> thread:
>   http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01998.html
>
> If we know we aren't going to be able to build executables, simply emit
> a warning saying that we can't link (instead of exiting with an error),
> and mention that we're assuming prctl does not exist for this target.
>
> Ok to commit?

Looks good to me (and sorry for not getting back on you)
I cannot approve it however.

-Andi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch, libiberty] Fix build regression
  2010-11-20 13:37     ` Ralf Wildenhues
@ 2010-11-21  8:47       ` Anthony Green
  0 siblings, 0 replies; 6+ messages in thread
From: Anthony Green @ 2010-11-21  8:47 UTC (permalink / raw)
  To: Ralf Wildenhues, gcc-patches

On 11/20/2010 7:11 AM, Ralf Wildenhues wrote:
> * Anthony Green wrote on Sat, Nov 20, 2010 at 01:04:59PM CET:
>> Thanks Ralf.  However last night on #gcc Ian questioned why this is
>> a link test in the first place.  Why not just check headers instead?
>> I've attached a revised patch based on header checking.
>>
>> Ok to commit?
>
> That looks fairly safe, and the idea seems good to me too.
> OK if it works (you can run 'make configure-libiberty' on a system with
> and one without the header to find out quickly).

Tested and committed.  Thanks!

AG



>
> Thanks,
> Ralf
>
>> 2010-11-19  Anthony Green<green@moxielogic.com>
>>
>> 	* configure.ac: Turn PR_SET_NAME link test into a test for
>> 	sys/prctl.h.
>> 	* configure, config.in: Rebuilt.
>> 	* setproctitle.c: Test for HAVE_SYS_PRCTL_H.
>> 	(setproctitle) Test for PR_SET_NAME definition.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-11-21  3:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-19 23:00 [Patch, libiberty] Fix build regression Anthony Green
2010-11-20 10:53 ` Ralf Wildenhues
2010-11-20 13:29   ` Anthony Green
2010-11-20 13:37     ` Ralf Wildenhues
2010-11-21  8:47       ` Anthony Green
2010-11-20 15:16 ` Andi Kleen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).