[PATCH, v2] Fix 32-bit build for --enable-targets=all

[PATCH, v2] Fix 32-bit build for --enable-targets=all

[Luis Machado]

v2:

- Enable disassembler support for bpf, cris, loongarch and tilegx for
  32-bit BFD.  This prevents crashes when GDB tries to find a disassembler
  for these architectures and runs into a null pointer.

--

The following fixes the GDB build for 32-bit (tested on 32-bit arm)
for the following combinations:

* --enable-targets=all --disable-sim
* --enable-targets=all

Previously I was seeing failures of this kind:

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43:
undefined reference to `bpf_cgen_cpu_close`

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166:
undefined reference to `bpf_cgen_cpu_open_1`

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179:
undefined reference to `bpf_cgen_init_dis`

This particular combination of switches has not been tested for 32-bit
hosts in a while (since November/December 2021), so there might be bugs
that we need to address. The patch makes things build cleanly though.

Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.

It would be nice to exercise this on other 32-bit targets, and get this
included in time for GDB 12.
---
 opcodes/Makefile.am   | 10 ++++++++++
 opcodes/Makefile.in   | 10 ++++++++++
 opcodes/disassemble.c |  4 ++++
 3 files changed, 24 insertions(+)

diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
index afd19fa7785..681fbc07584 100644
--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
 	arm-dis.c \
 	avr-dis.c \
 	bfin-dis.c \
+	bpf-asm.c \
+	bpf-desc.c \
+	bpf-dis.c \
+	bpf-ibld.c \
+	bpf-opc.c \
 	cgen-asm.c \
 	cgen-bitset.c \
 	cgen-dis.c \
@@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
 	lm32-ibld.c \
 	lm32-opc.c \
 	lm32-opinst.c \
+	loongarch-opc.c \
+	loongarch-dis.c \
+	loongarch-coder.c \
 	m10200-dis.c \
 	m10200-opc.c \
 	m10300-dis.c \
@@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
 	ppc-opc.c \
 	pru-dis.c \
 	pru-opc.c \
+	riscv-dis.c \
+	riscv-opc.c \
 	rl78-decode.c \
 	rl78-dis.c \
 	rx-decode.c \
diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
index 3ab8bfb0548..d3eee49b169 100644
--- a/opcodes/Makefile.in
+++ b/opcodes/Makefile.in
@@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
 	arm-dis.c \
 	avr-dis.c \
 	bfin-dis.c \
+	bpf-asm.c \
+	bpf-desc.c \
+	bpf-dis.c \
+	bpf-ibld.c \
+	bpf-opc.c \
 	cgen-asm.c \
 	cgen-bitset.c \
 	cgen-dis.c \
@@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
 	lm32-ibld.c \
 	lm32-opc.c \
 	lm32-opinst.c \
+	loongarch-opc.c \
+	loongarch-dis.c \
+	loongarch-coder.c \
 	m10200-dis.c \
 	m10200-opc.c \
 	m10300-dis.c \
@@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
 	ppc-opc.c \
 	pru-dis.c \
 	pru-opc.c \
+	riscv-dis.c \
+	riscv-opc.c \
 	rl78-decode.c \
 	rl78-dis.c \
 	rx-decode.c \
diff --git a/opcodes/disassemble.c b/opcodes/disassemble.c
index bd1b90b3956..7228df40ec0 100644
--- a/opcodes/disassemble.c
+++ b/opcodes/disassemble.c
@@ -42,7 +42,9 @@
 #define ARCH_arm
 #define ARCH_avr
 #define ARCH_bfin
+#define ARCH_bpf
 #define ARCH_cr16
+#define ARCH_cris
 #define ARCH_crx
 #define ARCH_csky
 #define ARCH_d10v
@@ -58,6 +60,7 @@
 #define ARCH_ip2k
 #define ARCH_iq2000
 #define ARCH_lm32
+#define ARCH_loongarch
 #define ARCH_m32c
 #define ARCH_m32r
 #define ARCH_m68hc11
@@ -92,6 +95,7 @@
 #define ARCH_tic4x
 #define ARCH_tic54x
 #define ARCH_tic6x
+#define ARCH_tilegx
 #define ARCH_tilepro
 #define ARCH_v850
 #define ARCH_vax
-- 
2.25.1

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Joel Brobecker]

Hi Luis,

On Fri, Apr 22, 2022 at 02:25:13PM +0100, Luis Machado wrote:
> v2:
> 
> - Enable disassembler support for bpf, cris, loongarch and tilegx for
>   32-bit BFD.  This prevents crashes when GDB tries to find a disassembler
>   for these architectures and runs into a null pointer.
> 
> --
> 
> The following fixes the GDB build for 32-bit (tested on 32-bit arm)
> for the following combinations:
> 
> * --enable-targets=all --disable-sim
> * --enable-targets=all
> 
> Previously I was seeing failures of this kind:
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43:
> undefined reference to `bpf_cgen_cpu_close`
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166:
> undefined reference to `bpf_cgen_cpu_open_1`
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179:
> undefined reference to `bpf_cgen_init_dis`
> 
> This particular combination of switches has not been tested for 32-bit
> hosts in a while (since November/December 2021), so there might be bugs
> that we need to address. The patch makes things build cleanly though.
> 
> Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.
> 
> It would be nice to exercise this on other 32-bit targets, and get this
> included in time for GDB 12.
> ---
>  opcodes/Makefile.am   | 10 ++++++++++
>  opcodes/Makefile.in   | 10 ++++++++++
>  opcodes/disassemble.c |  4 ++++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
> index afd19fa7785..681fbc07584 100644
> --- a/opcodes/Makefile.am
> +++ b/opcodes/Makefile.am
> @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
>  	arm-dis.c \
>  	avr-dis.c \
>  	bfin-dis.c \
> +	bpf-asm.c \
> +	bpf-desc.c \
> +	bpf-dis.c \
> +	bpf-ibld.c \
> +	bpf-opc.c \
>  	cgen-asm.c \
>  	cgen-bitset.c \
>  	cgen-dis.c \

Looking at this patch, I think you you may not have seen Alan's
comment, which je sent on Apr 18, saying:

    https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
    | Anything that requires 64-bit BFD support does not belong in
    | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
    | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
    | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
    | bpf here?  It's a 64-bit target!

(I see that you weren't in the list of direct recipients for that email)

> @@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
>  	lm32-ibld.c \
>  	lm32-opc.c \
>  	lm32-opinst.c \
> +	loongarch-opc.c \
> +	loongarch-dis.c \
> +	loongarch-coder.c \
>  	m10200-dis.c \
>  	m10200-opc.c \
>  	m10300-dis.c \
> @@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
>  	ppc-opc.c \
>  	pru-dis.c \
>  	pru-opc.c \
> +	riscv-dis.c \
> +	riscv-opc.c \
>  	rl78-decode.c \
>  	rl78-dis.c \
>  	rx-decode.c \
> diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
> index 3ab8bfb0548..d3eee49b169 100644
> --- a/opcodes/Makefile.in
> +++ b/opcodes/Makefile.in
> @@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
>  	arm-dis.c \
>  	avr-dis.c \
>  	bfin-dis.c \
> +	bpf-asm.c \
> +	bpf-desc.c \
> +	bpf-dis.c \
> +	bpf-ibld.c \
> +	bpf-opc.c \
>  	cgen-asm.c \
>  	cgen-bitset.c \
>  	cgen-dis.c \
> @@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
>  	lm32-ibld.c \
>  	lm32-opc.c \
>  	lm32-opinst.c \
> +	loongarch-opc.c \
> +	loongarch-dis.c \
> +	loongarch-coder.c \
>  	m10200-dis.c \
>  	m10200-opc.c \
>  	m10300-dis.c \
> @@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
>  	ppc-opc.c \
>  	pru-dis.c \
>  	pru-opc.c \
> +	riscv-dis.c \
> +	riscv-opc.c \
>  	rl78-decode.c \
>  	rl78-dis.c \
>  	rx-decode.c \
> diff --git a/opcodes/disassemble.c b/opcodes/disassemble.c
> index bd1b90b3956..7228df40ec0 100644
> --- a/opcodes/disassemble.c
> +++ b/opcodes/disassemble.c
> @@ -42,7 +42,9 @@
>  #define ARCH_arm
>  #define ARCH_avr
>  #define ARCH_bfin
> +#define ARCH_bpf
>  #define ARCH_cr16
> +#define ARCH_cris
>  #define ARCH_crx
>  #define ARCH_csky
>  #define ARCH_d10v
> @@ -58,6 +60,7 @@
>  #define ARCH_ip2k
>  #define ARCH_iq2000
>  #define ARCH_lm32
> +#define ARCH_loongarch
>  #define ARCH_m32c
>  #define ARCH_m32r
>  #define ARCH_m68hc11
> @@ -92,6 +95,7 @@
>  #define ARCH_tic4x
>  #define ARCH_tic54x
>  #define ARCH_tic6x
> +#define ARCH_tilegx
>  #define ARCH_tilepro
>  #define ARCH_v850
>  #define ARCH_vax
> -- 
> 2.25.1
> 

-- 
Joel

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Luis Machado]

Hi Joel,

On 4/24/22 15:58, Joel Brobecker wrote:
> Hi Luis,
> 
> On Fri, Apr 22, 2022 at 02:25:13PM +0100, Luis Machado wrote:
>> v2:
>>
>> - Enable disassembler support for bpf, cris, loongarch and tilegx for
>>    32-bit BFD.  This prevents crashes when GDB tries to find a disassembler
>>    for these architectures and runs into a null pointer.
>>
>> --
>>
>> The following fixes the GDB build for 32-bit (tested on 32-bit arm)
>> for the following combinations:
>>
>> * --enable-targets=all --disable-sim
>> * --enable-targets=all
>>
>> Previously I was seeing failures of this kind:
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43:
>> undefined reference to `bpf_cgen_cpu_close`
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166:
>> undefined reference to `bpf_cgen_cpu_open_1`
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179:
>> undefined reference to `bpf_cgen_init_dis`
>>
>> This particular combination of switches has not been tested for 32-bit
>> hosts in a while (since November/December 2021), so there might be bugs
>> that we need to address. The patch makes things build cleanly though.
>>
>> Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.
>>
>> It would be nice to exercise this on other 32-bit targets, and get this
>> included in time for GDB 12.
>> ---
>>   opcodes/Makefile.am   | 10 ++++++++++
>>   opcodes/Makefile.in   | 10 ++++++++++
>>   opcodes/disassemble.c |  4 ++++
>>   3 files changed, 24 insertions(+)
>>
>> diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
>> index afd19fa7785..681fbc07584 100644
>> --- a/opcodes/Makefile.am
>> +++ b/opcodes/Makefile.am
>> @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	arm-dis.c \
>>   	avr-dis.c \
>>   	bfin-dis.c \
>> +	bpf-asm.c \
>> +	bpf-desc.c \
>> +	bpf-dis.c \
>> +	bpf-ibld.c \
>> +	bpf-opc.c \
>>   	cgen-asm.c \
>>   	cgen-bitset.c \
>>   	cgen-dis.c \
> 
> Looking at this patch, I think you you may not have seen Alan's
> comment, which je sent on Apr 18, saying:
> 
>      https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
>      | Anything that requires 64-bit BFD support does not belong in
>      | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
>      | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
>      | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
>      | bpf here?  It's a 64-bit target!
> 
> (I see that you weren't in the list of direct recipients for that email)
> 

Yes, it looks that way.

Unfortunately --enable-targets=all never really worked OK for 32-bit 
builds after splitting 64/32 targets. It is not clear to me if there are 
bugs elsewhere that are preventing a clean build, but right now it 
doesn't look buildable at all.

Alan?


>> @@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	lm32-ibld.c \
>>   	lm32-opc.c \
>>   	lm32-opinst.c \
>> +	loongarch-opc.c \
>> +	loongarch-dis.c \
>> +	loongarch-coder.c \
>>   	m10200-dis.c \
>>   	m10200-opc.c \
>>   	m10300-dis.c \
>> @@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	ppc-opc.c \
>>   	pru-dis.c \
>>   	pru-opc.c \
>> +	riscv-dis.c \
>> +	riscv-opc.c \
>>   	rl78-decode.c \
>>   	rl78-dis.c \
>>   	rx-decode.c \
>> diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
>> index 3ab8bfb0548..d3eee49b169 100644
>> --- a/opcodes/Makefile.in
>> +++ b/opcodes/Makefile.in
>> @@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	arm-dis.c \
>>   	avr-dis.c \
>>   	bfin-dis.c \
>> +	bpf-asm.c \
>> +	bpf-desc.c \
>> +	bpf-dis.c \
>> +	bpf-ibld.c \
>> +	bpf-opc.c \
>>   	cgen-asm.c \
>>   	cgen-bitset.c \
>>   	cgen-dis.c \
>> @@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	lm32-ibld.c \
>>   	lm32-opc.c \
>>   	lm32-opinst.c \
>> +	loongarch-opc.c \
>> +	loongarch-dis.c \
>> +	loongarch-coder.c \
>>   	m10200-dis.c \
>>   	m10200-opc.c \
>>   	m10300-dis.c \
>> @@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
>>   	ppc-opc.c \
>>   	pru-dis.c \
>>   	pru-opc.c \
>> +	riscv-dis.c \
>> +	riscv-opc.c \
>>   	rl78-decode.c \
>>   	rl78-dis.c \
>>   	rx-decode.c \
>> diff --git a/opcodes/disassemble.c b/opcodes/disassemble.c
>> index bd1b90b3956..7228df40ec0 100644
>> --- a/opcodes/disassemble.c
>> +++ b/opcodes/disassemble.c
>> @@ -42,7 +42,9 @@
>>   #define ARCH_arm
>>   #define ARCH_avr
>>   #define ARCH_bfin
>> +#define ARCH_bpf
>>   #define ARCH_cr16
>> +#define ARCH_cris
>>   #define ARCH_crx
>>   #define ARCH_csky
>>   #define ARCH_d10v
>> @@ -58,6 +60,7 @@
>>   #define ARCH_ip2k
>>   #define ARCH_iq2000
>>   #define ARCH_lm32
>> +#define ARCH_loongarch
>>   #define ARCH_m32c
>>   #define ARCH_m32r
>>   #define ARCH_m68hc11
>> @@ -92,6 +95,7 @@
>>   #define ARCH_tic4x
>>   #define ARCH_tic54x
>>   #define ARCH_tic6x
>> +#define ARCH_tilegx
>>   #define ARCH_tilepro
>>   #define ARCH_v850
>>   #define ARCH_vax
>> -- 
>> 2.25.1
>>
> 

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Alan Modra]

On Mon, Apr 25, 2022 at 08:40:54AM +0100, Luis Machado wrote:
> On 4/24/22 15:58, Joel Brobecker wrote:
> > Looking at this patch, I think you you may not have seen Alan's
> > comment, which je sent on Apr 18, saying:
> > 
> >      https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
> >      | Anything that requires 64-bit BFD support does not belong in
> >      | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
> >      | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
> >      | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
> >      | bpf here?  It's a 64-bit target!
> > 
> > (I see that you weren't in the list of direct recipients for that email)
> > 
> 
> Yes, it looks that way.
> 
> Unfortunately --enable-targets=all never really worked OK for 32-bit builds
> after splitting 64/32 targets. It is not clear to me if there are bugs
> elsewhere that are preventing a clean build, but right now it doesn't look
> buildable at all.
> 
> Alan?

The major problem I have with your patch is that all it does is sweep
a problem under the rug.  While it may fix a build breakage I doubt
that it actually improves anything for users.  For example, if I apply
your patch for a 32-bit --enable-targets=all binutils build, then
attempt to disassemble one of the bpf gas testsuite objects:

$ ~/build/gas/all32/binutils/objdump -dr tmpdir/lddw.o
/home/alan/build/gas/all32/binutils/objdump: tmpdir/lddw.o: file format not recognized

That's due to lack of the required support from bfd/elf64-bpf.c to
load bpf object files into BFD.  I think you'll find a similar result
for the other targets your patch touches, and not just with objdump
but with everything else that uses libbfd.

There is also a minor problem with the patch in that it adds entries
to TARGET32_LIBOPCODES_CFILES without removing the corresponding
entries from TARGET64_LIBOPCODES_CFILES.  Similarly for the defines in
opcodes/disassemble.c.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Luis Machado]

On 4/26/22 03:52, Alan Modra wrote:
> On Mon, Apr 25, 2022 at 08:40:54AM +0100, Luis Machado wrote:
>> On 4/24/22 15:58, Joel Brobecker wrote:
>>> Looking at this patch, I think you you may not have seen Alan's
>>> comment, which je sent on Apr 18, saying:
>>>
>>>       https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
>>>       | Anything that requires 64-bit BFD support does not belong in
>>>       | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
>>>       | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
>>>       | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
>>>       | bpf here?  It's a 64-bit target!
>>>
>>> (I see that you weren't in the list of direct recipients for that email)
>>>
>>
>> Yes, it looks that way.
>>
>> Unfortunately --enable-targets=all never really worked OK for 32-bit builds
>> after splitting 64/32 targets. It is not clear to me if there are bugs
>> elsewhere that are preventing a clean build, but right now it doesn't look
>> buildable at all.
>>
>> Alan?
> 
> The major problem I have with your patch is that all it does is sweep
> a problem under the rug.  While it may fix a build breakage I doubt
> that it actually improves anything for users.  For example, if I apply
> your patch for a 32-bit --enable-targets=all binutils build, then
> attempt to disassemble one of the bpf gas testsuite objects:
> 
> $ ~/build/gas/all32/binutils/objdump -dr tmpdir/lddw.o
> /home/alan/build/gas/all32/binutils/objdump: tmpdir/lddw.o: file format not recognized
>

Yeah, I agree. Doing some more research on the code, it is the wrong 
approach. Though I'm wondering what an appropriate fix would look like, 
one that wouldn't require a lot of work and would still make things 
build cleanly. A compromise?

> That's due to lack of the required support from bfd/elf64-bpf.c to
> load bpf object files into BFD.  I think you'll find a similar result
> for the other targets your patch touches, and not just with objdump
> but with everything else that uses libbfd.
> 
> There is also a minor problem with the patch in that it adds entries
> to TARGET32_LIBOPCODES_CFILES without removing the corresponding
> entries from TARGET64_LIBOPCODES_CFILES.  Similarly for the defines in
> opcodes/disassemble.c.
>

Maybe some targets shouldn't be accepted as part of a 32-bit 
--enable-targets=all for GDB/sim. Removing those "problematic" targets 
from the list might be another possible approach I suppose.

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Jose E. Marchesi]

Hi Alan.

> On Mon, Apr 25, 2022 at 08:40:54AM +0100, Luis Machado wrote:
>> On 4/24/22 15:58, Joel Brobecker wrote:
>> > Looking at this patch, I think you you may not have seen Alan's
>> > comment, which je sent on Apr 18, saying:
>> > 
>> >      https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
>> >      | Anything that requires 64-bit BFD support does not belong in
>> >      | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
>> >      | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
>> >      | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
>> >      | bpf here?  It's a 64-bit target!
>> > 
>> > (I see that you weren't in the list of direct recipients for that email)
>> > 
>> 
>> Yes, it looks that way.
>> 
>> Unfortunately --enable-targets=all never really worked OK for 32-bit builds
>> after splitting 64/32 targets. It is not clear to me if there are bugs
>> elsewhere that are preventing a clean build, but right now it doesn't look
>> buildable at all.
>> 
>> Alan?
>
> The major problem I have with your patch is that all it does is sweep
> a problem under the rug.  While it may fix a build breakage I doubt
> that it actually improves anything for users.  For example, if I apply
> your patch for a 32-bit --enable-targets=all binutils build, then
> attempt to disassemble one of the bpf gas testsuite objects:
>
> $ ~/build/gas/all32/binutils/objdump -dr tmpdir/lddw.o
> /home/alan/build/gas/all32/binutils/objdump: tmpdir/lddw.o: file format not recognized
>
> That's due to lack of the required support from bfd/elf64-bpf.c to
> load bpf object files into BFD.

Could you please elaborate on that?

What is it in elf64-bpf.c that must be improved in order to support
32-bit --enable-targets=all binutils?

> I think you'll find a similar result for the other targets your patch
> touches, and not just with objdump but with everything else that uses
> libbfd.
>
> There is also a minor problem with the patch in that it adds entries
> to TARGET32_LIBOPCODES_CFILES without removing the corresponding
> entries from TARGET64_LIBOPCODES_CFILES.  Similarly for the defines in
> opcodes/disassemble.c.

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Alan Modra]

On Tue, Apr 26, 2022 at 07:31:07AM +0100, Luis Machado wrote:
> Maybe some targets shouldn't be accepted as part of a 32-bit
> --enable-targets=all for GDB/sim. Removing those "problematic" targets from
> the list might be another possible approach I suppose.

Exactly.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Alan Modra]

On Tue, Apr 26, 2022 at 10:07:53AM +0200, Jose E. Marchesi wrote:
> 
> Hi Alan.
> 
> > On Mon, Apr 25, 2022 at 08:40:54AM +0100, Luis Machado wrote:
> >> On 4/24/22 15:58, Joel Brobecker wrote:
> >> > Looking at this patch, I think you you may not have seen Alan's
> >> > comment, which je sent on Apr 18, saying:
> >> > 
> >> >      https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
> >> >      | Anything that requires 64-bit BFD support does not belong in
> >> >      | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
> >> >      | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
> >> >      | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
> >> >      | bpf here?  It's a 64-bit target!
> >> > 
> >> > (I see that you weren't in the list of direct recipients for that email)
> >> > 
> >> 
> >> Yes, it looks that way.
> >> 
> >> Unfortunately --enable-targets=all never really worked OK for 32-bit builds
> >> after splitting 64/32 targets. It is not clear to me if there are bugs
> >> elsewhere that are preventing a clean build, but right now it doesn't look
> >> buildable at all.
> >> 
> >> Alan?
> >
> > The major problem I have with your patch is that all it does is sweep
> > a problem under the rug.  While it may fix a build breakage I doubt
> > that it actually improves anything for users.  For example, if I apply
> > your patch for a 32-bit --enable-targets=all binutils build, then
> > attempt to disassemble one of the bpf gas testsuite objects:
> >
> > $ ~/build/gas/all32/binutils/objdump -dr tmpdir/lddw.o
> > /home/alan/build/gas/all32/binutils/objdump: tmpdir/lddw.o: file format not recognized
> >
> > That's due to lack of the required support from bfd/elf64-bpf.c to
> > load bpf object files into BFD.
> 
> Could you please elaborate on that?
> 
> What is it in elf64-bpf.c that must be improved in order to support
> 32-bit --enable-targets=all binutils?

It isn't that elf64-bpf.c is missing something, it's that the entire
file is not built on a 32-bit host with --enable-targets=all and
without --enable-64-bit-bfd.

> > I think you'll find a similar result for the other targets your patch
> > touches, and not just with objdump but with everything else that uses
> > libbfd.
> >
> > There is also a minor problem with the patch in that it adds entries
> > to TARGET32_LIBOPCODES_CFILES without removing the corresponding
> > entries from TARGET64_LIBOPCODES_CFILES.  Similarly for the defines in
> > opcodes/disassemble.c.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Pedro Alves]

On 2022-04-26 07:31, Luis Machado via Binutils wrote:
>>
> 
> Maybe some targets shouldn't be accepted as part of a 32-bit --enable-targets=all for GDB/sim. Removing those "problematic" targets from the list might be another possible approach I suppose.

That is already the case.  See ALL_64_TARGET_OBS vs ALL_TARGET_OBS in gdb/Makefile.in:

 # All target-dependent objects files that require 64-bit CORE_ADDR
 # (used with --enable-targets=all --enable-64-bit-bfd).
 ALL_64_TARGET_OBS = \

 # All other target-dependent objects files (used with --enable-targets=all).
 ALL_TARGET_OBS = \


Sounds like you'd just want to move bpf-tdep.o from ALL_TARGET_OBS to ALL_64_TARGET_OBS.

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Luis Machado]

On 4/27/22 10:57, Pedro Alves wrote:
> On 2022-04-26 07:31, Luis Machado via Binutils wrote:
>>>
>>
>> Maybe some targets shouldn't be accepted as part of a 32-bit --enable-targets=all for GDB/sim. Removing those "problematic" targets from the list might be another possible approach I suppose.
> 
> That is already the case.  See ALL_64_TARGET_OBS vs ALL_TARGET_OBS in gdb/Makefile.in:
> 
>   # All target-dependent objects files that require 64-bit CORE_ADDR
>   # (used with --enable-targets=all --enable-64-bit-bfd).
>   ALL_64_TARGET_OBS = \
> 
>   # All other target-dependent objects files (used with --enable-targets=all).
>   ALL_TARGET_OBS = \
> 
> 
> Sounds like you'd just want to move bpf-tdep.o from ALL_TARGET_OBS to ALL_64_TARGET_OBS.

Ah, thanks. I'll give that a try. There might be other misplaced entries.

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Luis Machado]

On 4/26/22 14:22, Alan Modra wrote:
> On Tue, Apr 26, 2022 at 10:07:53AM +0200, Jose E. Marchesi wrote:
>>
>> Hi Alan.
>>
>>> On Mon, Apr 25, 2022 at 08:40:54AM +0100, Luis Machado wrote:
>>>> On 4/24/22 15:58, Joel Brobecker wrote:
>>>>> Looking at this patch, I think you you may not have seen Alan's
>>>>> comment, which je sent on Apr 18, saying:
>>>>>
>>>>>       https://sourceware.org/pipermail/gdb-patches/2022-April/187960.html
>>>>>       | Anything that requires 64-bit BFD support does not belong in
>>>>>       | TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
>>>>>       | TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
>>>>>       | 32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
>>>>>       | bpf here?  It's a 64-bit target!
>>>>>
>>>>> (I see that you weren't in the list of direct recipients for that email)
>>>>>
>>>>
>>>> Yes, it looks that way.
>>>>
>>>> Unfortunately --enable-targets=all never really worked OK for 32-bit builds
>>>> after splitting 64/32 targets. It is not clear to me if there are bugs
>>>> elsewhere that are preventing a clean build, but right now it doesn't look
>>>> buildable at all.
>>>>
>>>> Alan?
>>>
>>> The major problem I have with your patch is that all it does is sweep
>>> a problem under the rug.  While it may fix a build breakage I doubt
>>> that it actually improves anything for users.  For example, if I apply
>>> your patch for a 32-bit --enable-targets=all binutils build, then
>>> attempt to disassemble one of the bpf gas testsuite objects:
>>>
>>> $ ~/build/gas/all32/binutils/objdump -dr tmpdir/lddw.o
>>> /home/alan/build/gas/all32/binutils/objdump: tmpdir/lddw.o: file format not recognized
>>>
>>> That's due to lack of the required support from bfd/elf64-bpf.c to
>>> load bpf object files into BFD.
>>
>> Could you please elaborate on that?
>>
>> What is it in elf64-bpf.c that must be improved in order to support
>> 32-bit --enable-targets=all binutils?
> 
> It isn't that elf64-bpf.c is missing something, it's that the entire
> file is not built on a 32-bit host with --enable-targets=all and
> without --enable-64-bit-bfd.
> 
>>> I think you'll find a similar result for the other targets your patch
>>> touches, and not just with objdump but with everything else that uses
>>> libbfd.
>>>
>>> There is also a minor problem with the patch in that it adds entries
>>> to TARGET32_LIBOPCODES_CFILES without removing the corresponding
>>> entries from TARGET64_LIBOPCODES_CFILES.  Similarly for the defines in
>>> opcodes/disassemble.c.
> 

After some investigation, it seems the bpf target is a bit of a corner 
case. It's within the 64-bit bfd group, so libopcodes gets built only if 
--enable-64-bit-bfd. Otherwise, libopcodes doesn't include bpf.

The bpf sim gets built regardless of having a 32-bit bfd or 64-bit bfd, 
so in the case of a 32-bit build with --enable-targets=all (and no 
--enable-64-bit-bfd), libopcodes doesn't include bpf, causing a libsim 
linking failure due to missing symbols (.

Things work fine for 64-bit though. I think the fix would involve not 
building the sim if the bpf files are not linked into libopcodes.

--

libsim.a(sim-close.o): In function `sim_close':
/builds/binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43: 
undefined reference to `bpf_cgen_cpu_close'
libsim.a(sim-if.o): In function `sim_open':
/builds/binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166: 
undefined reference to `bpf_cgen_cpu_open_1'
/builds/binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179: 
undefined reference to `bpf_cgen_init_dis'
collect2: error: ld returned 1 exit status

Re: [PATCH, v2] Fix 32-bit build for --enable-targets=all

[Pedro Alves]

On 2022-04-29 13:35, Luis Machado via Binutils wrote:

> 
> After some investigation, it seems the bpf target is a bit of a corner case. It's within the 64-bit bfd group, so libopcodes gets built only if --enable-64-bit-bfd. Otherwise, libopcodes doesn't include bpf.
> 
> The bpf sim gets built regardless of having a 32-bit bfd or 64-bit bfd, so in the case of a 32-bit build with --enable-targets=all (and no --enable-64-bit-bfd), libopcodes doesn't include bpf, causing a libsim linking failure due to missing symbols (.
> 
> Things work fine for 64-bit though. I think the fix would involve not building the sim if the bpf files are not linked into libopcodes.

Can't the sim m4_include bfd64.m4 too and check enable_64_bit_bfd like opcodes, gdb, ld, etc. do?