Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
Alan Modra via Binutils <[email protected]> Fri, 26 Nov 2021 09:32:11 +1030
| Newsgroups | gmane.comp.gnu.binutils,gmane.comp.tools.cgen.devel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Sep 15, 2021 at 06:04:44PM +0000, Guillermo Martinez wrote:
> On Wednesday, September 15, 2021 11:49:12 AM CDT Jose E. Marchesi wrote:
> > I just installed this version of the patch on your behalf.
> > Thanks!
> >
> > > If an instruction field is defined in a long form, assigning
> > > an offset different to 0 the mask and constant values are not
> > > computed appropriately:
[snip]
It's been a while since I refreshed the binutils opcodes files that
are cgen generated. On doing so it appears this patch is responsible
for build errors when using mainline gcc. First example:
/home/alan/src/binutils-gdb/opcodes/bpf-opc.c:57:11: error: conversion from ‘long unsigned int’ to ‘unsigned int’ changes value from ‘18446744073709486335’ to ‘4294902015’ [-Werror=overflow]
57 | 64, 64, 0xffffffffffff00ff, { { F (F_IMM32) }, { F (F_OFFSET16) }, { F (F_SRCLE) }, { F (F_OP_CODE) }, { F (F_DSTLE) }, { F (F_OP_SRC) }, { F (F_OP_CLASS) }, { 0 } }
| ^~~~~~~~~~~~~~~~~~
I tested an obvious workaround,
diff --git a/include/opcode/cgen.h b/include/opcode/cgen.h
index 8b7d2a4b547..0e9571d19b0 100644
--- a/include/opcode/cgen.h
+++ b/include/opcode/cgen.h
@@ -914,7 +914,7 @@ typedef struct
Each insn's value is stored with the insn.
The first step in recognizing an insn for disassembly is
(opcode & mask) == value. */
- CGEN_INSN_INT mask;
+ uint64_t mask;
#define CGEN_IFMT_MASK(ifmt) ((ifmt)->mask)
/* Instruction fields.
diff --git a/opcodes/cgen-dis.c b/opcodes/cgen-dis.c
index 1a5d1ae8459..37ee5a23564 100644
--- a/opcodes/cgen-dis.c
+++ b/opcodes/cgen-dis.c
@@ -39,7 +39,7 @@ static void add_insn_to_hash_chain (CGEN_INSN_LIST *,
static int
count_decodable_bits (const CGEN_INSN *insn)
{
- unsigned mask = CGEN_INSN_BASE_MASK (insn);
+ uint64_t mask = CGEN_INSN_BASE_MASK (insn);
#if GCC_VERSION >= 3004
return __builtin_popcount (mask);
#else
Quite possibly the above isn't a complete fix, I just threw it
together to see what happens. Regressions:
+FAIL: eBPF CALL instruction
+FAIL: eBPF CALL instruction, big endian
+FAIL: CALL with disp32 reloc
+FAIL: CALL with disp32 reloc and addend
+FAIL: CALL check unsigned underflow
gas/testsuite/gas.log for the first one shows:
regexp_diff match failure
regexp "^ 20: 85 10 00 00 00 00 00 00 call 0$"
line " 20: 85 10 00 00 00 00 00 00 *unknown*"
regexp_diff match failure
regexp "^ 28: 85 10 00 00 ff ff ff ff call -1$"
line " 28: 85 10 00 00 ff ff ff ff *unknown*"
regexp_diff match failure
regexp "^ 30: 85 10 00 00 fe ff ff ff call -2$"
line " 30: 85 10 00 00 fe ff ff ff *unknown*"
regexp_diff match failure
regexp "^ 38: 85 10 00 00 fd ff ff ff call -3$"
line " 38: 85 10 00 00 fd ff ff ff *unknown*"
FAIL: eBPF CALL instruction
At this point I don't intend to look any further into the problem.
The opcodes/ refresh can wait.
--
Alan Modra
Australia Development Lab, IBM