public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB
       [not found]   ` <wwok1sx9nhet.fsf@ericsson.com>
@ 2016-12-15 20:10     ` Maciej W. Rozycki
  2016-12-15 23:03       ` Alan Modra
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-15 20:10 UTC (permalink / raw)
  To: Antoine Tremblay; +Cc: sergiodj+buildbot, gdb-patches, binutils

On Thu, 15 Dec 2016, Antoine Tremblay wrote:

> >  This failure is very odd to me, it looks like `opcodes/mips-dis.c' has 
> > been included in the build of `libopcodes.a', however `bfd/elfxx-mips.c' 
> > has *not* been included in the build of `libbfd.a'.  Offhand I would 
> > consider such a configuration broken, however maybe it is legitimate after 
> > all.  Has the opcodes/ subdirectory been configured differently from the 
> > bfd/ subdirectory by any chance?
> 
> I would look for:
> http://gdb-build.sergiodj.net/builders/Debian-i686-native-extended-gdbserver/builds/4691/steps/configure%20gdb/logs/stdio
> and
> http://gdb-build.sergiodj.net/builders/Debian-i686-native-extended-gdbserver/builds/4691/steps/compile%20gdb/logs/stdio
> 
> For all the configure options.

 Thanks, `--build=i686-pc-linux-gnu' does trigger the problem for me as 
well, and AFAICT the underlying issue is MIPS target support is not 
included in BFD as it wants 64-bit BFD which is not enabled, however the 
opcodes library is still built.

 Obviously such a configuration is useless for `libopcodes' as you can't 
get at all the target-specific particulars BFD would normally provide, so 
not even the binutils/ subdirectory tools (excluded, by request, from this 
configuration, though otherwise buildable) can correctly support the MIPS 
target, let alone GDB.

 So I think we need to arrange for the list of targets enabled for other 
subdirectories to be driven somehow by BFD or, more likely, the top level.
I'm not sure offhand how to do this though.  Cc-ing `binutils' for wider 
audience.

 I'll see if there is something I could do right away as a temporary 
measure to unbreak 32-bit BFD configurations -- I would make the reference 
from `opcodes/mips-dis.c' to `bfd_mips_elf_get_abiflags' weak, however 
regrettably this does not appear supported, so maybe we'll require a dummy 
stub or suchlike hackery if MIPS target support is enabled, but not 
included in BFD.

 Any better suggestions?

  Maciej

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

* Re: Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB
  2016-12-15 20:10     ` Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB Maciej W. Rozycki
@ 2016-12-15 23:03       ` Alan Modra
  2016-12-15 23:19         ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Alan Modra @ 2016-12-15 23:03 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Antoine Tremblay, sergiodj+buildbot, gdb-patches, binutils

On Thu, Dec 15, 2016 at 08:09:42PM +0000, Maciej W. Rozycki wrote:
>  I'll see if there is something I could do right away as a temporary 
> measure to unbreak 32-bit BFD configurations -- I would make the reference 
> from `opcodes/mips-dis.c' to `bfd_mips_elf_get_abiflags' weak, however 
> regrettably this does not appear supported, so maybe we'll require a dummy 
> stub or suchlike hackery if MIPS target support is enabled, but not 
> included in BFD.

Make the new ELF code in mips-dis.c conditional on BFD64?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB
  2016-12-15 23:03       ` Alan Modra
@ 2016-12-15 23:19         ` Maciej W. Rozycki
  2016-12-19 11:49           ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-15 23:19 UTC (permalink / raw)
  To: Alan Modra; +Cc: Antoine Tremblay, sergiodj+buildbot, gdb-patches, binutils

On Thu, 15 Dec 2016, Alan Modra wrote:

> >  I'll see if there is something I could do right away as a temporary 
> > measure to unbreak 32-bit BFD configurations -- I would make the reference 
> > from `opcodes/mips-dis.c' to `bfd_mips_elf_get_abiflags' weak, however 
> > regrettably this does not appear supported, so maybe we'll require a dummy 
> > stub or suchlike hackery if MIPS target support is enabled, but not 
> > included in BFD.
> 
> Make the new ELF code in mips-dis.c conditional on BFD64?

 Ah, that sounds like the right direction, although I'd rather exclude all 
MIPS target support (Score wants that too, based on their code) at the 
`configure' level.  It looks like we handle that already in ld/, by 
interpreting `want64' and pulling `../bfd/config.bfd' if required, so I'll 
see if I can import that into binutils/ as well.  Thanks for the hint!

  Maciej

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

* Re: Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB
  2016-12-15 23:19         ` Maciej W. Rozycki
@ 2016-12-19 11:49           ` Maciej W. Rozycki
       [not found]             ` <20161222123958.GA2896@bubble.grove.modra.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-19 11:49 UTC (permalink / raw)
  To: Alan Modra; +Cc: Antoine Tremblay, sergiodj+buildbot, gdb-patches, binutils

Hi Alan,

On Thu, 15 Dec 2016, Maciej W. Rozycki wrote:

> On Thu, 15 Dec 2016, Alan Modra wrote:
> 
> > >  I'll see if there is something I could do right away as a temporary 
> > > measure to unbreak 32-bit BFD configurations -- I would make the reference 
> > > from `opcodes/mips-dis.c' to `bfd_mips_elf_get_abiflags' weak, however 
> > > regrettably this does not appear supported, so maybe we'll require a dummy 
> > > stub or suchlike hackery if MIPS target support is enabled, but not 
> > > included in BFD.
> > 
> > Make the new ELF code in mips-dis.c conditional on BFD64?
> 
>  Ah, that sounds like the right direction, although I'd rather exclude all 
> MIPS target support (Score wants that too, based on their code) at the 
> `configure' level.  It looks like we handle that already in ld/, by 
> interpreting `want64' and pulling `../bfd/config.bfd' if required, so I'll 
> see if I can import that into binutils/ as well.  Thanks for the hint!

 So I have followed your advice after all and made a minimal change (see 
commit 4df995c77118 ("MIPS/opcodes: Only call `bfd_mips_elf_get_abiflags' 
if BFD64")), because in such a configuration we may still have other MIPS 
BFD targets configured, such as ECOFF, so the disassembler has to remain 
fully functional for `objdump' at least.

  Maciej

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

* [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
       [not found]             ` <20161222123958.GA2896@bubble.grove.modra.org>
@ 2016-12-23 12:11               ` Maciej W. Rozycki
  2016-12-23 14:01                 ` Alan Modra
  2016-12-27 10:08                 ` Joel Brobecker
  0 siblings, 2 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-23 12:11 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

Fix a regression introduced with commit 5e7fc731f80e ("MIPS/opcodes: 
Also set disassembler's ASE flags from ELF structures"), further updated 
with commit 4df995c77118 ("MIPS/opcodes: Also set disassembler's ASE 
flags from ELF structures"), and use autoconf to check for the presence 
of `bfd_mips_elf_get_abiflags' in BFD.

	opcodes/
	* mips-dis.c (set_default_mips_dis_options): Use 
	HAVE_BFD_MIPS_ELF_GET_ABIFLAGS rather than BFD64 to guard the
	call to `bfd_mips_elf_get_abiflags'.
	* configure.ac: Check for `bfd_mips_elf_get_abiflags' in BFD.
	* Makefile.am (CONFIG_STATUS_DEPENDENCIES): Add `libbfd.la'.
	* aclocal.m4: Regenerate.
	* configure: Regenerate.
	* config.in: Regenerate.
	* Makefile.in: Regenerate.
---
On Thu, 22 Dec 2016, Alan Modra wrote:

> On Mon, Dec 19, 2016 at 11:49:23AM +0000, Maciej W. Rozycki wrote:
> >  So I have followed your advice after all and made a minimal change (see 
> > commit 4df995c77118 ("MIPS/opcodes: Only call `bfd_mips_elf_get_abiflags' 
> > if BFD64")), because in such a configuration we may still have other MIPS 
> > BFD targets configured, such as ECOFF, so the disassembler has to remain 
> > fully functional for `objdump' at least.
> 
> Unfortunately this fails.  For --build=x86_64-linux --enable-plugins
> --disable-gdb --disable-sim --disable-readline --disable-libdecnumber
> --enable-obsolete --target=mips-ecoff
> 
> ../opcodes/.libs/libopcodes.a(mips-dis.o): In function `set_default_mips_dis_options':
> /home/alan/src/binutils-gdb/opcodes/mips-dis.c:855: undefined reference to `bfd_mips_elf_get_abiflags'
> collect2: error: ld returned 1 exit status
> Makefile:819: recipe for target 'objdump' failed

 Hmm, I dropped `mips-ecoff' and `mipsel-ecoff' as test targets for being 
obsolete as GAS support was removed a while ago, but this situation proves 
it was premature as they at least need to continue building even if we 
don't care about regressions anymore.  Obviously it would have triggered 
and brought my attention even before I pushed my problematic change.  I 
put them back then.

 Let's deal with the breakage the proper way and use autoconf to check for 
this symbol, which is what autoconf has been made for.  There is a slight 
complication with the convoluted way we wire BFD into the link, but that's
nothing that couldn't be dealt with.  OTOH I don't think this internal API 
is worth abstracting in a target-independent way via a generic BFD entry 
point.

 No regressions against the usual 164 targets (exact list available upon 
request), `mips-ecoff' and `mipsel-ecoff' targets now build successfully, 
and the original failing native `i686-pc-linux-gnu' configuration still 
builds including binutils/ and gdb/ subdirectories in particular.

 OK to apply then?

  Maciej

binutils-opcodes-mips-abiflags-autoconf.diff
Index: binutils/opcodes/Makefile.am
===================================================================
--- binutils.orig/opcodes/Makefile.am	2016-12-20 18:41:24.000000000 +0000
+++ binutils/opcodes/Makefile.am	2016-12-23 01:07:40.302417847 +0000
@@ -300,8 +300,9 @@ ALL_MACHINES = $(TARGET_LIBOPCODES_CFILE
 
 OFILES = @BFD_MACHINES@
 
-# development.sh is used to determine -Werror default.
-CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
+# development.sh is used to determine -Werror default, libbfd.la is needed
+# for function availability checks.
+CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh ../bfd/libbfd.la
 
 AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(INCDIR) -I$(BFDDIR) @HDEFINES@ @INCINTL@
 
Index: binutils/opcodes/Makefile.in
===================================================================
--- binutils.orig/opcodes/Makefile.in	2016-12-20 18:41:24.000000000 +0000
+++ binutils/opcodes/Makefile.in	2016-12-23 01:07:43.708319394 +0000
@@ -80,7 +80,9 @@ DIST_COMMON = ChangeLog $(srcdir)/Makefi
 	$(top_srcdir)/po/Make-in $(srcdir)/../depcomp
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../bfd/acinclude.m4 \
-	$(top_srcdir)/../bfd/warning.m4 $(top_srcdir)/../config/acx.m4 \
+	$(top_srcdir)/../bfd/warning.m4 \
+	$(top_srcdir)/../config/acinclude.m4 \
+	$(top_srcdir)/../config/acx.m4 \
 	$(top_srcdir)/../config/depstand.m4 \
 	$(top_srcdir)/../config/gettext-sister.m4 \
 	$(top_srcdir)/../config/lead-dot.m4 \
@@ -187,6 +189,8 @@ AUTOCONF = @AUTOCONF@
 AUTOHEADER = @AUTOHEADER@
 AUTOMAKE = @AUTOMAKE@
 AWK = @AWK@
+BFDHDIR = @BFDHDIR@
+BFDLIB = @BFDLIB@
 BFD_MACHINES = @BFD_MACHINES@
 BUILD_LIBS = @BUILD_LIBS@
 BUILD_LIB_DEPS = @BUILD_LIB_DEPS@
@@ -603,8 +607,9 @@ CFILES = \
 ALL_MACHINES = $(TARGET_LIBOPCODES_CFILES:.c=.lo)
 OFILES = @BFD_MACHINES@
 
-# development.sh is used to determine -Werror default.
-CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
+# development.sh is used to determine -Werror default, libbfd.la is needed
+# for function availability checks.
+CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh ../bfd/libbfd.la
 AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(INCDIR) -I$(BFDDIR) @HDEFINES@ @INCINTL@
 libopcodes_la_SOURCES = dis-buf.c disassemble.c dis-init.c
 # It's desirable to list ../bfd/libbfd.la in DEPENDENCIES and LIBADD.
Index: binutils/opcodes/aclocal.m4
===================================================================
--- binutils.orig/opcodes/aclocal.m4	2016-12-22 20:11:50.376586695 +0000
+++ binutils/opcodes/aclocal.m4	2016-12-22 20:14:39.423005866 +0000
@@ -990,6 +990,7 @@ AC_SUBST([am__untar])
 
 m4_include([../bfd/acinclude.m4])
 m4_include([../bfd/warning.m4])
+m4_include([../config/acinclude.m4])
 m4_include([../config/acx.m4])
 m4_include([../config/depstand.m4])
 m4_include([../config/gettext-sister.m4])
Index: binutils/opcodes/config.in
===================================================================
--- binutils.orig/opcodes/config.in	2016-12-22 20:11:50.214485476 +0000
+++ binutils/opcodes/config.in	2016-12-22 20:14:39.439141818 +0000
@@ -11,6 +11,9 @@
    language is requested. */
 #undef ENABLE_NLS
 
+/* Define to 1 if you have the `bfd_mips_elf_get_abiflags' function. */
+#undef HAVE_BFD_MIPS_ELF_GET_ABIFLAGS
+
 /* Define to 1 if you have the declaration of `basename', and to 0 if you
    don't. */
 #undef HAVE_DECL_BASENAME
Index: binutils/opcodes/configure
===================================================================
--- binutils.orig/opcodes/configure	2016-12-22 20:11:50.300991961 +0000
+++ binutils/opcodes/configure	2016-12-23 01:57:34.234831514 +0000
@@ -604,6 +604,8 @@ LTLIBOBJS
 LIBOBJS
 BFD_MACHINES
 archdefs
+BFDLIB
+BFDHDIR
 SHARED_DEPENDENCIES
 SHARED_LIBADD
 SHARED_LDFLAGS
@@ -11151,7 +11153,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11154 "configure"
+#line 11156 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11257,7 +11259,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11260 "configure"
+#line 11262 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12586,6 +12588,100 @@ fi
 
 # target-specific stuff:
 
+# Check if `bfd_mips_elf_get_abiflags' is present in BFD, needed by
+# `mips-dis.c'.  Avoid using AC_CHECK_LIB as it uses a cache variable
+# which could hold the wrong value if we are rerun due to the
+# `../bfd/libbfd.la' dependency for `config.status', so use a handcoded
+# sequence which is equivalent but does not use the cache.
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for the bfd header in the build tree" >&5
+$as_echo_n "checking for the bfd header in the build tree... " >&6; }
+dirlist=".. ../../ ../../../ ../../../../ ../../../../../ ../../../../../../ ../../../../../../.. ../../../../../../../.. ../../../../../../../../.. ../../../../../../../../../.."
+if test "${ac_cv_c_bfdh+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+
+for i in $dirlist; do
+    if test -f "$i/bfd/bfd.h" ; then
+	ac_cv_c_bfdh=`(cd $i/bfd; ${PWDCMD-pwd})`
+	break
+    fi
+done
+
+fi
+
+if test x"${ac_cv_c_bfdh}" != x; then
+    BFDHDIR="-I${ac_cv_c_bfdh}"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: ${ac_cv_c_bfdh}" >&5
+$as_echo "${ac_cv_c_bfdh}" >&6; }
+else
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: none" >&5
+$as_echo "none" >&6; }
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for the bfd library in the build tree" >&5
+$as_echo_n "checking for the bfd library in the build tree... " >&6; }
+if test "${ac_cv_c_bfdlib+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+
+for i in $dirlist; do
+    if test -f "$i/bfd/Makefile" ; then
+	ac_cv_c_bfdlib=`(cd $i/bfd; ${PWDCMD-pwd})`
+    fi
+done
+
+fi
+
+if test x"${ac_cv_c_bfdlib}" != x; then
+    BFDLIB="-L${ac_cv_c_bfdlib} -L${ac_cv_c_bfdlib}/.libs"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: ${ac_cv_c_bfdlib}" >&5
+$as_echo "${ac_cv_c_bfdlib}" >&6; }
+else
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: none" >&5
+$as_echo "none" >&6; }
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for bfd_mips_elf_get_abiflags in -lbfd" >&5
+$as_echo_n "checking for bfd_mips_elf_get_abiflags in -lbfd... " >&6; }
+opcodes_save_LIBS=$LIBS
+LIBS="$BFDLIB -lbfd ../libiberty/libiberty.a `test -e ../bfd/libbfd.la && . ../bfd/libbfd.la; echo $dependency_libs`"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char bfd_mips_elf_get_abiflags ();
+int
+main ()
+{
+return bfd_mips_elf_get_abiflags ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  opcodes_lib_bfd_bfd_mips_elf_get_abiflags=yes
+else
+  opcodes_lib_bfd_bfd_mips_elf_get_abiflags=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+LIBS=$opcodes_save_LIBS
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $opcodes_lib_bfd_bfd_mips_elf_get_abiflags" >&5
+$as_echo "$opcodes_lib_bfd_bfd_mips_elf_get_abiflags" >&6; }
+if test x"$opcodes_lib_bfd_bfd_mips_elf_get_abiflags" = xyes; then
+
+$as_echo "#define HAVE_BFD_MIPS_ELF_GET_ABIFLAGS 1" >>confdefs.h
+
+fi
+
 # Canonicalize the secondary target names.
 if test -n "$enable_targets" ; then
     for targ in `echo $enable_targets | sed 's/,/ /g'`
Index: binutils/opcodes/configure.ac
===================================================================
--- binutils.orig/opcodes/configure.ac	2016-12-22 20:11:50.305044953 +0000
+++ binutils/opcodes/configure.ac	2016-12-23 01:57:26.538274592 +0000
@@ -210,6 +210,25 @@ AC_SUBST(SHARED_DEPENDENCIES)
 
 # target-specific stuff:
 
+# Check if `bfd_mips_elf_get_abiflags' is present in BFD, needed by
+# `mips-dis.c'.  Avoid using AC_CHECK_LIB as it uses a cache variable
+# which could hold the wrong value if we are rerun due to the
+# `../bfd/libbfd.la' dependency for `config.status', so use a handcoded
+# sequence which is equivalent but does not use the cache.
+CYG_AC_PATH_BFD
+AC_MSG_CHECKING([for bfd_mips_elf_get_abiflags in -lbfd])
+opcodes_save_LIBS=$LIBS
+LIBS="$BFDLIB -lbfd ../libiberty/libiberty.a `test -e ../bfd/libbfd.la && . ../bfd/libbfd.la; echo $dependency_libs`"
+AC_LINK_IFELSE([AC_LANG_CALL([], [bfd_mips_elf_get_abiflags])],
+               [opcodes_lib_bfd_bfd_mips_elf_get_abiflags=yes],
+               [opcodes_lib_bfd_bfd_mips_elf_get_abiflags=no])
+LIBS=$opcodes_save_LIBS
+AC_MSG_RESULT([$opcodes_lib_bfd_bfd_mips_elf_get_abiflags])
+if test x"$opcodes_lib_bfd_bfd_mips_elf_get_abiflags" = xyes; then
+  AC_DEFINE([HAVE_BFD_MIPS_ELF_GET_ABIFLAGS], [1],
+    [Define to 1 if you have the `bfd_mips_elf_get_abiflags' function.])
+fi
+
 # Canonicalize the secondary target names.
 if test -n "$enable_targets" ; then
     for targ in `echo $enable_targets | sed 's/,/ /g'`
Index: binutils/opcodes/mips-dis.c
===================================================================
--- binutils.orig/opcodes/mips-dis.c	2016-12-22 20:11:52.872833905 +0000
+++ binutils/opcodes/mips-dis.c	2016-12-22 20:14:39.478580342 +0000
@@ -847,11 +847,12 @@ set_default_mips_dis_options (struct dis
       Elf_Internal_Ehdr *header = elf_elfheader (abfd);
       Elf_Internal_ABIFlags_v0 *abiflags = NULL;
 
-      /* We won't ever get here if !BFD64, because we won't then have
-	 a MIPS/ELF BFD, however we need to guard against a link error
-	 in a `--enable-targets=...' configuration with a 32-bit host,
-	 where the MIPS target is a secondary.  */
-#ifdef BFD64
+      /* We won't ever get here if !HAVE_BFD_MIPS_ELF_GET_ABIFLAGS,
+	 because we won't then have a MIPS/ELF BFD, however we need
+	 to guard against a link error in a `--enable-targets=...'
+	 configuration with a 32-bit host where the MIPS target is
+	 a secondary, or with MIPS/ECOFF configurations.  */
+#ifdef HAVE_BFD_MIPS_ELF_GET_ABIFLAGS
       abiflags = bfd_mips_elf_get_abiflags (abfd);
 #endif
       /* If an ELF "newabi" binary, use the n32/(n)64 GPR names.  */

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-23 12:11               ` [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD Maciej W. Rozycki
@ 2016-12-23 14:01                 ` Alan Modra
  2016-12-23 20:45                   ` Maciej W. Rozycki
  2016-12-27 10:08                 ` Joel Brobecker
  1 sibling, 1 reply; 11+ messages in thread
From: Alan Modra @ 2016-12-23 14:01 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils

On Fri, Dec 23, 2016 at 12:11:12PM +0000, Maciej W. Rozycki wrote:
> 	* mips-dis.c (set_default_mips_dis_options): Use 
> 	HAVE_BFD_MIPS_ELF_GET_ABIFLAGS rather than BFD64 to guard the
> 	call to `bfd_mips_elf_get_abiflags'.
> 	* configure.ac: Check for `bfd_mips_elf_get_abiflags' in BFD.
> 	* Makefile.am (CONFIG_STATUS_DEPENDENCIES): Add `libbfd.la'.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* Makefile.in: Regenerate.

Looks OK to me.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-23 14:01                 ` Alan Modra
@ 2016-12-23 20:45                   ` Maciej W. Rozycki
  0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-23 20:45 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On Fri, 23 Dec 2016, Alan Modra wrote:

> > 	* mips-dis.c (set_default_mips_dis_options): Use 
> > 	HAVE_BFD_MIPS_ELF_GET_ABIFLAGS rather than BFD64 to guard the
> > 	call to `bfd_mips_elf_get_abiflags'.
> > 	* configure.ac: Check for `bfd_mips_elf_get_abiflags' in BFD.
> > 	* Makefile.am (CONFIG_STATUS_DEPENDENCIES): Add `libbfd.la'.
> > 	* aclocal.m4: Regenerate.
> > 	* configure: Regenerate.
> > 	* config.in: Regenerate.
> > 	* Makefile.in: Regenerate.
> 
> Looks OK to me.

 Great, thanks for a quick review!  Applied now and backported to 2.28.

  Maciej

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-23 12:11               ` [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD Maciej W. Rozycki
  2016-12-23 14:01                 ` Alan Modra
@ 2016-12-27 10:08                 ` Joel Brobecker
  2016-12-28 12:11                   ` Alan Modra
  2016-12-31  9:15                   ` Maciej W. Rozycki
  1 sibling, 2 replies; 11+ messages in thread
From: Joel Brobecker @ 2016-12-27 10:08 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Alan Modra, binutils, gdb-patches, Tristan Gingold

> Fix a regression introduced with commit 5e7fc731f80e ("MIPS/opcodes: 
> Also set disassembler's ASE flags from ELF structures"), further updated 
> with commit 4df995c77118 ("MIPS/opcodes: Also set disassembler's ASE 
> flags from ELF structures"), and use autoconf to check for the presence 
> of `bfd_mips_elf_get_abiflags' in BFD.
> 
> 	opcodes/
> 	* mips-dis.c (set_default_mips_dis_options): Use 
> 	HAVE_BFD_MIPS_ELF_GET_ABIFLAGS rather than BFD64 to guard the
> 	call to `bfd_mips_elf_get_abiflags'.
> 	* configure.ac: Check for `bfd_mips_elf_get_abiflags' in BFD.
> 	* Makefile.am (CONFIG_STATUS_DEPENDENCIES): Add `libbfd.la'.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* Makefile.in: Regenerate.

Unfortunately, this change breaks the following scenario, used
by the src-release.sh script (used to produce our nightly source
packages, as well as our official releases). It also looks like
the change is in the binutils 2.28 branch as well, so if Tristan
uses that script to produce the release, it's not going to work.

The reason for the failure is the following change:

   -# development.sh is used to determine -Werror default.
   -CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
   +# development.sh is used to determine -Werror default, libbfd.la is needed
   +# for function availability checks.
   +CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh ../bfd/libbfd.la

It causes the following scenario to fail:

   $ ./configure
   $ make configure-host
   $ make distclean

I'm pretty sure "./configure; make; make distclean" fails the same way,
but I haven't bothered trying.

The reason it fails is that "make distclean" depends on distclean-host
which then depends on:

    distclean-host: maybe-distclean-bfd
    distclean-host: maybe-distclean-opcodes

So, "make distclean" first does a "distclean" in bfd, followed by
a "distclean" in opcodes. The bfd distclean results in libbfd.la
being deleted. And because opcodes' Makefile now depends on it,
we get this error:

| make: Entering directory '/[...]/obj/opcodes'
| make: *** No rule to make target '../bfd/libbfd.la', needed by 'config.status'.  Stop.
| make: Leaving directory '/[...]/obj/opcodes'

I haven't been able to figure out how distclean depends on
CONFIG_STATUS_DEPENDENCIES, but I'm thiking it's probably one of
the implicit dependencies. But looking at the automake documentation
about CONFIG_STATUS_DEPENDENCIES, it looks like this is meant to be
listing the depedencies to re-generate the Makefile. It don't think
libbfd.a/la is in this category, is it?

I don't have a solution. Perhaps one way to approach the problem
might be to distclean in the reverse order to configure/build?
If opcodes depends on bfd and we distclean bfd, then there might
indeed be some dependencies missing.

We need a solution fairly quickly...

-- 
Joel

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-27 10:08                 ` Joel Brobecker
@ 2016-12-28 12:11                   ` Alan Modra
  2016-12-30  6:44                     ` Joel Brobecker
  2016-12-31  9:15                   ` Maciej W. Rozycki
  1 sibling, 1 reply; 11+ messages in thread
From: Alan Modra @ 2016-12-28 12:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Maciej W. Rozycki, binutils, gdb-patches, Tristan Gingold

On Tue, Dec 27, 2016 at 02:08:41PM +0400, Joel Brobecker wrote:
> The reason for the failure is the following change:
> 
>    -# development.sh is used to determine -Werror default.
>    -CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
>    +# development.sh is used to determine -Werror default, libbfd.la is needed
>    +# for function availability checks.
>    +CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh ../bfd/libbfd.la
> 
> It causes the following scenario to fail:
> 
>    $ ./configure
>    $ make configure-host
>    $ make distclean
> 
> I'm pretty sure "./configure; make; make distclean" fails the same way,

Yes, it does.  So let's revert that patch and simply modify the make
rule for mips-dis.lo (ie. provide it to overrided the default automake
rule) to test whether elfxx-mips.c has been compiled in.  The top
level makefile already has the required directory dependencies. 

	* configure.ac: Revert 2016-12-23.
	* Makefile.am: Likewise.
	(MIPS_DEFS): Define.
	(mips-dis.lo): Add rule.
	* Makefile.in: Regenerate.
	* aclocal.m4: Regenerate.
	* config.in: Regenerate.
	* configure: Regenerate.

Diff below excludes the reversion.

diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
index 3e9dc54..a441feb 100644
--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -610,6 +609,19 @@ $(srcdir)/z8k-opc.h: @MAINT@ z8kgen$(EXEEXT_FOR_BUILD)
 
 z8k-dis.lo: $(srcdir)/z8k-opc.h
 
+MIPS_DEFS=`case \`cat ../bfd/ofiles\` in *elfxx-mips*) echo "-DHAVE_BFD_MIPS_ELF_GET_ABIFLAGS=1";; esac`
+mips-dis.lo: mips-dis.c
+if am__fastdepCC
+	$(LTCOMPILE) -MT $@ -MD -MP -MF $(DEPDIR)/$*.Tpo -c -o $@ $(MIPS_DEFS) $<
+	$(am__mv) $(DEPDIR)/$*.Tpo $(DEPDIR)/$*.Plo
+else
+if AMDEP
+	source='$<' object='$@' libtool=yes @AMDEPBACKSLASH@
+	DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
+endif
+	$(LTCOMPILE) -c -o $@ $(MIPS_DEFS) $<
+endif
+
 sh-dis.lo: sh-dis.c
 if am__fastdepCC
 	$(LTCOMPILE) -MT $@ -MD -MP -MF $(DEPDIR)/$*.Tpo -c -o $@ @archdefs@ $(srcdir)/sh-dis.c

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-28 12:11                   ` Alan Modra
@ 2016-12-30  6:44                     ` Joel Brobecker
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2016-12-30  6:44 UTC (permalink / raw)
  To: Alan Modra; +Cc: Maciej W. Rozycki, binutils, gdb-patches, Tristan Gingold

> Yes, it does.  So let's revert that patch and simply modify the make
> rule for mips-dis.lo (ie. provide it to overrided the default automake
> rule) to test whether elfxx-mips.c has been compiled in.  The top
> level makefile already has the required directory dependencies. 
> 
> 	* configure.ac: Revert 2016-12-23.
> 	* Makefile.am: Likewise.
> 	(MIPS_DEFS): Define.
> 	(mips-dis.lo): Add rule.
> 	* Makefile.in: Regenerate.
> 	* aclocal.m4: Regenerate.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.

Thanks, Alan!

-- 
Joel

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

* Re: [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD
  2016-12-27 10:08                 ` Joel Brobecker
  2016-12-28 12:11                   ` Alan Modra
@ 2016-12-31  9:15                   ` Maciej W. Rozycki
  1 sibling, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2016-12-31  9:15 UTC (permalink / raw)
  To: Joel Brobecker, Alan Modra
  Cc: Maciej W. Rozycki, binutils, gdb-patches, Tristan Gingold

On Tue, 27 Dec 2016, Joel Brobecker wrote:

> I haven't been able to figure out how distclean depends on
> CONFIG_STATUS_DEPENDENCIES, but I'm thiking it's probably one of
> the implicit dependencies. But looking at the automake documentation
> about CONFIG_STATUS_DEPENDENCIES, it looks like this is meant to be
> listing the depedencies to re-generate the Makefile. It don't think
> libbfd.a/la is in this category, is it?

 It is not, however `config.status' itself is, and with the change I made 
it depended on `libbfd.la' because one of the autoconf tests did.  So if 
e.g. you ran `configure' in opcodes/ first (for whatever reason), then 
running `make' there would run `configure' and then `make' in bfd/ because 
of other dependencies, however `config.status' would not get updated in 
opcodes/ afterwards, before its targets were made.

> I don't have a solution. Perhaps one way to approach the problem
> might be to distclean in the reverse order to configure/build?
> If opcodes depends on bfd and we distclean bfd, then there might
> indeed be some dependencies missing.
> 
> We need a solution fairly quickly...

 Thanks, Alan, for sorting this out in my absence!

 TBH I think `distclean' shouldn't really depend on non-phony targets in 
the first place, or at least it should ignore errors in their re-creation, 
but there you go.  With releases upcoming it's no time now to revamp our 
build system!

 Happy New Year!

  Maciej

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

end of thread, other threads:[~2016-12-31  9:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <E1cHNui-0007Mp-46@kwanyin.sergiodj.net>
     [not found] ` <alpine.DEB.2.00.1612150502490.6743@tp.orcam.me.uk>
     [not found]   ` <wwok1sx9nhet.fsf@ericsson.com>
2016-12-15 20:10     ` Your commit 'MIPS/opcodes: Also set disassembler's ASE flags from ELF structures' broke GDB Maciej W. Rozycki
2016-12-15 23:03       ` Alan Modra
2016-12-15 23:19         ` Maciej W. Rozycki
2016-12-19 11:49           ` Maciej W. Rozycki
     [not found]             ` <20161222123958.GA2896@bubble.grove.modra.org>
2016-12-23 12:11               ` [PATCH, RFA] opcodes: Use autoconf to check for `bfd_mips_elf_get_abiflags' in BFD Maciej W. Rozycki
2016-12-23 14:01                 ` Alan Modra
2016-12-23 20:45                   ` Maciej W. Rozycki
2016-12-27 10:08                 ` Joel Brobecker
2016-12-28 12:11                   ` Alan Modra
2016-12-30  6:44                     ` Joel Brobecker
2016-12-31  9:15                   ` Maciej W. Rozycki

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).