Patchwork [top-level] Do proper target tool checks for readelf

login
register
mail settings
Submitter Roland McGrath
Date Nov. 2, 2011, 10:48 p.m.
Message ID <x57j8vnyywcb.fsf@frobland.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/123357/
State New
Headers show

Comments

Roland McGrath - Nov. 2, 2011, 10:48 p.m.
newlib has a configure check that works by running readelf on a target
binary.  While a native readelf is generally pretty generic and
cross-friendly, it's possible for a build host not to have any readelf at
all.  It's clearly the right thing that configure use a target readelf tool
if there is one.  Right now, this seems to happen for the "main" newlib
build but doesn't happen for a multilib variant newlib build (e.g. in an
x86_64-foo config, build/x86_64-foo/newlib/... uses a target readelf but
build/x86_64-foo/32/newlib/... doesn't find one and so uses unadorned
"readelf", which might not exist on the build host in a cross situation).

Frankly I couldn't entirely tell which of all these magic spots were the
ones necessary to get the multilib newlib build to do the right thing.  But
it makes sense that the treatment of readelf be uniformly analogous to the
treatment of objdump, so I did that everywhere I found it.

MAINTAINERS doesn't make it entirely clear what the procedure really is for
touching these files.  Looking at src/ChangeLog I see both what appear to
be instances where changes were first committed to GCC and then merged over
to src/, and instances where changes were committed directly to src/.  I'm
able to commit (after approval) to src/ CVS, but not to GCC.

Ok for src/ trunk, or should somebody be putting this into GCC first?

I've omitted the generated files from the patch, but of course would
include them in a commit.


Thanks,
Roland

2011-11-02  Roland McGrath  <mcgrathr@google.com>

	* configure.ac: Add tool checks for READELF and READELF_FOR_TARGET.
	* configure: Rebuild.
	* Makefile.def (flags_to_pass): Add READELF_FOR_TARGET.
	* Makefile.tpl (READELF, READELF_FOR_TARGET): New variables.
	(HOST_EXPORTS): Add READELF, READELF_FOR_TARGET.
	(BASE_FLAGS_TO_PASS): Add READELF_FOR_TARGET.
	(BASE_TARGET_EXPORTS, EXTRA_HOST_FLAGS, EXTRA_TARGET_FLAGS):
	Add READELF.
	* Makefile.in: Rebuild.
Ian Taylor - Nov. 3, 2011, 3:32 a.m.
Roland McGrath <mcgrathr@google.com> writes:

> MAINTAINERS doesn't make it entirely clear what the procedure really is for
> touching these files.  Looking at src/ChangeLog I see both what appear to
> be instances where changes were first committed to GCC and then merged over
> to src/, and instances where changes were committed directly to src/.  I'm
> able to commit (after approval) to src/ CVS, but not to GCC.

The top level is not automatically merged between gcc and src.  However,
people are expected to merge manually.  Normally gcc is considered to be
the master.

> Ok for src/ trunk, or should somebody be putting this into GCC first?

So, yes, should probably go into gcc first.

Ian
Roland McGrath - Nov. 3, 2011, 4:34 p.m.
On Wed, Nov 2, 2011 at 8:32 PM, Ian Lance Taylor <iant@google.com> wrote:
> The top level is not automatically merged between gcc and src.  However,
> people are expected to merge manually.  Normally gcc is considered to be
> the master.

Ok.  src/MAINTAINERS could be clearer about that.

>> Ok for src/ trunk, or should somebody be putting this into GCC first?
>
> So, yes, should probably go into gcc first.

Ok.  Would you like to commit it for me, then?  I can take care of the src/
merge myself if you prefer.


Thanks,
Roland
Ian Taylor - Nov. 3, 2011, 5:39 p.m.
Roland McGrath <mcgrathr@google.com> writes:

> On Wed, Nov 2, 2011 at 8:32 PM, Ian Lance Taylor <iant@google.com> wrote:
>> The top level is not automatically merged between gcc and src.  However,
>> people are expected to merge manually.  Normally gcc is considered to be
>> the master.
>
> Ok.  src/MAINTAINERS could be clearer about that.
>
>>> Ok for src/ trunk, or should somebody be putting this into GCC first?
>>
>> So, yes, should probably go into gcc first.
>
> Ok.  Would you like to commit it for me, then?  I can take care of the src/
> merge myself if you prefer.

Ideally I'd like to see a build maintainer approve it.  Perhaps I missed
that.

Ian
Roland McGrath - Nov. 3, 2011, 5:52 p.m.
On Thu, Nov 3, 2011 at 10:39 AM, Ian Lance Taylor <iant@google.com> wrote:
> Ideally I'd like to see a build maintainer approve it.  Perhaps I missed
> that.

Ah.  The src/MAINTAINERS file says, "Any global maintainer can approve
changes to these files, ...", which in that context I think includes you.
But since "..." is "but they should be aware that they need to be kept in
sync with their counterparts in the GCC repository," I've now looked at
gcc/MAINTAINERS and see that it has a category "build machinery (*.in)"
that you are not in.  (You are in its "global reviewers" category, which
the wording there suggests is in fact sufficient.  But I certainly can't
argue with wanting to hear from one of the build maintainers, though I do
hope they respond soon.  That said, it seems hard to imagine that my change
would be objectionable.)


Thanks,
Roland
DJ Delorie - Nov. 3, 2011, 5:55 p.m.
> Ideally I'd like to see a build maintainer approve it.  Perhaps I missed
> that.

I've got my usual paranoia about command line lengths being stressed,
but I suspect we're already way past the limits on the ones that will
actually fail.

The patch looks OK to me.
Roland McGrath - Nov. 3, 2011, 6:05 p.m.
On Thu, Nov 3, 2011 at 10:55 AM, DJ Delorie <dj@redhat.com> wrote:
> The patch looks OK to me.

Thanks!  As I'm still not a GCC committer, someone please check it in for me.
If people would like me to handle the merge over to src/ myself, I'd be
glad to do that step.


Thanks,
Roland
Roland McGrath - Nov. 9, 2011, 6:35 p.m.
On Thu, Nov 3, 2011 at 11:05 AM, Roland McGrath <mcgrathr@google.com> wrote:
> On Thu, Nov 3, 2011 at 10:55 AM, DJ Delorie <dj@redhat.com> wrote:
>> The patch looks OK to me.
>
> Thanks!  As I'm still not a GCC committer, someone please check it in for me.
> If people would like me to handle the merge over to src/ myself, I'd be
> glad to do that step.

Ping.  Anybody going to do this commit for me?  (If insteaed someone would
like to add me to the gcc group so I can do "write after approval", that
would be fine too.)


Thanks,
Roland
DJ Delorie - Nov. 9, 2011, 6:57 p.m.
> Ping.  Anybody going to do this commit for me?  (If insteaed someone would
> like to add me to the gcc group so I can do "write after approval", that
> would be fine too.)

Done.

Patch

diff --git a/Makefile.def b/Makefile.def
index 5116341..27434d3 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -259,6 +259,7 @@  flags_to_pass = { flag= LIBCXXFLAGS_FOR_TARGET ; };
 flags_to_pass = { flag= NM_FOR_TARGET ; };
 flags_to_pass = { flag= OBJDUMP_FOR_TARGET ; };
 flags_to_pass = { flag= RANLIB_FOR_TARGET ; };
+flags_to_pass = { flag= READELF_FOR_TARGET ; };
 flags_to_pass = { flag= STRIP_FOR_TARGET ; };
 flags_to_pass = { flag= WINDRES_FOR_TARGET ; };
 flags_to_pass = { flag= WINDMC_FOR_TARGET ; };
diff --git a/Makefile.tpl b/Makefile.tpl
index 4dd2391..a42d154 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -6,7 +6,7 @@  in
 #
 # Makefile for directory with subdirs to build.
 #   Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-#   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
+#   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011
 #   Free Software Foundation
 #
 # This file is free software; you can redistribute it and/or modify
@@ -209,6 +209,7 @@  HOST_EXPORTS = \
 	WINDMC="$(WINDMC)"; export WINDMC; \
 	OBJCOPY="$(OBJCOPY)"; export OBJCOPY; \
 	OBJDUMP="$(OBJDUMP)"; export OBJDUMP; \
+	READELF="$(READELF)"; export READELF; \
 	AR_FOR_TARGET="$(AR_FOR_TARGET)"; export AR_FOR_TARGET; \
 	AS_FOR_TARGET="$(AS_FOR_TARGET)"; export AS_FOR_TARGET; \
 	GCC_FOR_TARGET="$(GCC_FOR_TARGET)"; export GCC_FOR_TARGET; \
@@ -216,6 +217,7 @@  HOST_EXPORTS = \
 	NM_FOR_TARGET="$(NM_FOR_TARGET)"; export NM_FOR_TARGET; \
 	OBJDUMP_FOR_TARGET="$(OBJDUMP_FOR_TARGET)"; export OBJDUMP_FOR_TARGET; \
 	RANLIB_FOR_TARGET="$(RANLIB_FOR_TARGET)"; export RANLIB_FOR_TARGET; \
+	READELF_FOR_TARGET="$(READELF_FOR_TARGET)"; export READELF_FOR_TARGET; \
 	TOPLEVEL_CONFIGURE_ARGUMENTS="$(TOPLEVEL_CONFIGURE_ARGUMENTS)"; export TOPLEVEL_CONFIGURE_ARGUMENTS; \
 	HOST_LIBS="$(STAGE1_LIBS)"; export HOST_LIBS; \
 	GMPLIBS="$(HOST_GMPLIBS)"; export GMPLIBS; \
@@ -288,6 +290,7 @@  BASE_TARGET_EXPORTS = \
 	NM="$(COMPILER_NM_FOR_TARGET)"; export NM; \
 	OBJDUMP="$(OBJDUMP_FOR_TARGET)"; export OBJDUMP; \
 	RANLIB="$(RANLIB_FOR_TARGET)"; export RANLIB; \
+	READELF="$(READELF_FOR_TARGET)"; export READELF; \
 	STRIP="$(STRIP_FOR_TARGET)"; export STRIP; \
 	WINDRES="$(WINDRES_FOR_TARGET)"; export WINDRES; \
 	WINDMC="$(WINDMC_FOR_TARGET)"; export WINDMC; \
@@ -400,6 +403,7 @@  LIPO = @LIPO@
 NM = @NM@
 OBJDUMP = @OBJDUMP@
 RANLIB = @RANLIB@
+READELF = @READELF@
 STRIP = @STRIP@
 WINDRES = @WINDRES@
 WINDMC = @WINDMC@
@@ -493,6 +497,7 @@  LIPO_FOR_TARGET=@LIPO_FOR_TARGET@
 NM_FOR_TARGET=@NM_FOR_TARGET@
 OBJDUMP_FOR_TARGET=@OBJDUMP_FOR_TARGET@
 RANLIB_FOR_TARGET=@RANLIB_FOR_TARGET@
+READELF_FOR_TARGET=@READELF_FOR_TARGET@
 STRIP_FOR_TARGET=@STRIP_FOR_TARGET@
 WINDRES_FOR_TARGET=@WINDRES_FOR_TARGET@
 WINDMC_FOR_TARGET=@WINDMC_FOR_TARGET@
@@ -612,6 +617,7 @@  EXTRA_HOST_FLAGS = \
 	'NM=$(NM)' \
 	'OBJDUMP=$(OBJDUMP)' \
 	'RANLIB=$(RANLIB)' \
+	'READELF=$(READELF)' \
 	'STRIP=$(STRIP)' \
 	'WINDRES=$(WINDRES)' \
 	'WINDMC=$(WINDMC)'
@@ -652,6 +658,7 @@  EXTRA_TARGET_FLAGS = \
 	'NM=$(COMPILER_NM_FOR_TARGET)' \
 	'OBJDUMP=$$(OBJDUMP_FOR_TARGET)' \
 	'RANLIB=$$(RANLIB_FOR_TARGET)' \
+	'READELF=$$(READELF_FOR_TARGET)' \
 	'WINDRES=$$(WINDRES_FOR_TARGET)' \
 	'WINDMC=$$(WINDMC_FOR_TARGET)' \
 	'XGCC_FLAGS_FOR_TARGET=$(XGCC_FLAGS_FOR_TARGET)' \
diff --git a/configure.ac b/configure.ac
index 337e11d..50907f3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3042,6 +3042,7 @@  NCN_STRICT_CHECK_TOOLS(WINDRES, windres)
 NCN_STRICT_CHECK_TOOLS(WINDMC, windmc)
 NCN_STRICT_CHECK_TOOLS(OBJCOPY, objcopy)
 NCN_STRICT_CHECK_TOOLS(OBJDUMP, objdump)
+NCN_STRICT_CHECK_TOOLS(READELF, readelf)
 AC_SUBST(CC)
 AC_SUBST(CXX)
 AC_SUBST(CFLAGS)
@@ -3075,6 +3076,7 @@  ACX_CHECK_INSTALLED_TARGET_TOOL(LIPO_FOR_TARGET, lipo)
 ACX_CHECK_INSTALLED_TARGET_TOOL(NM_FOR_TARGET, nm)
 ACX_CHECK_INSTALLED_TARGET_TOOL(OBJDUMP_FOR_TARGET, objdump)
 ACX_CHECK_INSTALLED_TARGET_TOOL(RANLIB_FOR_TARGET, ranlib)
+ACX_CHECK_INSTALLED_TARGET_TOOL(READELF_FOR_TARGET, readelf)
 ACX_CHECK_INSTALLED_TARGET_TOOL(STRIP_FOR_TARGET, strip)
 ACX_CHECK_INSTALLED_TARGET_TOOL(WINDRES_FOR_TARGET, windres)
 ACX_CHECK_INSTALLED_TARGET_TOOL(WINDMC_FOR_TARGET, windmc)
@@ -3104,6 +3106,7 @@  GCC_TARGET_TOOL(lipo, LIPO_FOR_TARGET, LIPO)
 GCC_TARGET_TOOL(nm, NM_FOR_TARGET, NM, [binutils/nm-new])
 GCC_TARGET_TOOL(objdump, OBJDUMP_FOR_TARGET, OBJDUMP, [binutils/objdump])
 GCC_TARGET_TOOL(ranlib, RANLIB_FOR_TARGET, RANLIB, [binutils/ranlib])
+GCC_TARGET_TOOL(readelf, READELF_FOR_TARGET, READELF, [binutils/readelf])
 GCC_TARGET_TOOL(strip, STRIP_FOR_TARGET, STRIP, [binutils/strip-new])
 GCC_TARGET_TOOL(windres, WINDRES_FOR_TARGET, WINDRES, [binutils/windres])
 GCC_TARGET_TOOL(windmc, WINDMC_FOR_TARGET, WINDMC, [binutils/windmc])