* [committed 0/2] MIPS: Don't use a 32-bit architecture with a 64-bit ABI
@ 2018-02-26 19:48 Maciej W. Rozycki
2018-02-26 19:49 ` [committed 1/2] MIPS: Reorder ABI determination ahead of target description loading Maciej W. Rozycki
2018-02-26 19:50 ` [committed 2/2] MIPS: Don't use a 32-bit BFD architecture with a 64-bit ABI Maciej W. Rozycki
0 siblings, 2 replies; 3+ messages in thread
From: Maciej W. Rozycki @ 2018-02-26 19:48 UTC (permalink / raw)
To: gdb-patches
Hi,
This mini patch series addresses an issue on an internal error when an
attempt is made to use a 32-bit architecture with a 64-bit ABI. This is
clearly an unsupported combination and we should handle it gracefully.
A preparatory change is required so that the ABI in effect is determined
before a target description is loaded. This is because we change
architecture information and that is needed to initialise OS dependent
register information, which in turn target description loading relies on.
It actually seems cleaner to do this in this order regardless.
This has been extensively regression-tested with o32, n32 and n64 ABIs in
both native and remote configurations each, and now committed.
See individual change descriptions for details.
Maciej
^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed 1/2] MIPS: Reorder ABI determination ahead of target description loading
2018-02-26 19:48 [committed 0/2] MIPS: Don't use a 32-bit architecture with a 64-bit ABI Maciej W. Rozycki
@ 2018-02-26 19:49 ` Maciej W. Rozycki
2018-02-26 19:50 ` [committed 2/2] MIPS: Don't use a 32-bit BFD architecture with a 64-bit ABI Maciej W. Rozycki
1 sibling, 0 replies; 3+ messages in thread
From: Maciej W. Rozycki @ 2018-02-26 19:49 UTC (permalink / raw)
To: gdb-patches
Move ABI determination code ahead of target description loading so that
architecture information can be adjusted according to the ABI selected,
and then used in OS dependent register information initialization needed
for target description processing. No functional change.
gdb/
* gdb/mips-tdep.c (mips_gdbarch_init): Reorder ABI determination
ahead of target description loading.
---
gdb/mips-tdep.c | 360 +++++++++++++++++++++++++++-----------------------------
1 file changed, 178 insertions(+), 182 deletions(-)
gdb-mips-gdbarch-init-arches-abi-isa-ahead-tdesc.diff
Index: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c 2017-12-14 10:26:13.000000000 +0000
+++ binutils/gdb/mips-tdep.c 2017-12-14 10:32:34.439700558 +0000
@@ -8084,185 +8084,6 @@ mips_gdbarch_init (struct gdbarch_info i
int dspacc;
int dspctl;
- /* Fill in the OS dependent register numbers and names. */
- if (info.osabi == GDB_OSABI_LINUX)
- {
- mips_regnum.fp0 = 38;
- mips_regnum.pc = 37;
- mips_regnum.cause = 36;
- mips_regnum.badvaddr = 35;
- mips_regnum.hi = 34;
- mips_regnum.lo = 33;
- mips_regnum.fp_control_status = 70;
- mips_regnum.fp_implementation_revision = 71;
- mips_regnum.dspacc = -1;
- mips_regnum.dspctl = -1;
- dspacc = 72;
- dspctl = 78;
- num_regs = 90;
- reg_names = mips_linux_reg_names;
- }
- else
- {
- mips_regnum.lo = MIPS_EMBED_LO_REGNUM;
- mips_regnum.hi = MIPS_EMBED_HI_REGNUM;
- mips_regnum.badvaddr = MIPS_EMBED_BADVADDR_REGNUM;
- mips_regnum.cause = MIPS_EMBED_CAUSE_REGNUM;
- mips_regnum.pc = MIPS_EMBED_PC_REGNUM;
- mips_regnum.fp0 = MIPS_EMBED_FP0_REGNUM;
- mips_regnum.fp_control_status = 70;
- mips_regnum.fp_implementation_revision = 71;
- mips_regnum.dspacc = dspacc = -1;
- mips_regnum.dspctl = dspctl = -1;
- num_regs = MIPS_LAST_EMBED_REGNUM + 1;
- if (info.bfd_arch_info != NULL
- && info.bfd_arch_info->mach == bfd_mach_mips3900)
- reg_names = mips_tx39_reg_names;
- else
- reg_names = mips_generic_reg_names;
- }
-
- /* Check any target description for validity. */
- if (tdesc_has_registers (info.target_desc))
- {
- static const char *const mips_gprs[] = {
- "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
- "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
- "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
- "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"
- };
- static const char *const mips_fprs[] = {
- "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
- "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
- "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
- "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
- };
-
- const struct tdesc_feature *feature;
- int valid_p;
-
- feature = tdesc_find_feature (info.target_desc,
- "org.gnu.gdb.mips.cpu");
- if (feature == NULL)
- return NULL;
-
- tdesc_data = tdesc_data_alloc ();
-
- valid_p = 1;
- for (i = MIPS_ZERO_REGNUM; i <= MIPS_RA_REGNUM; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
- mips_gprs[i]);
-
-
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.lo, "lo");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.hi, "hi");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.pc, "pc");
-
- if (!valid_p)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- feature = tdesc_find_feature (info.target_desc,
- "org.gnu.gdb.mips.cp0");
- if (feature == NULL)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- valid_p = 1;
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.badvaddr, "badvaddr");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- MIPS_PS_REGNUM, "status");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.cause, "cause");
-
- if (!valid_p)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- /* FIXME drow/2007-05-17: The FPU should be optional. The MIPS
- backend is not prepared for that, though. */
- feature = tdesc_find_feature (info.target_desc,
- "org.gnu.gdb.mips.fpu");
- if (feature == NULL)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- valid_p = 1;
- for (i = 0; i < 32; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- i + mips_regnum.fp0, mips_fprs[i]);
-
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.fp_control_status,
- "fcsr");
- valid_p
- &= tdesc_numbered_register (feature, tdesc_data,
- mips_regnum.fp_implementation_revision,
- "fir");
-
- if (!valid_p)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- num_regs = mips_regnum.fp_implementation_revision + 1;
-
- if (dspacc >= 0)
- {
- feature = tdesc_find_feature (info.target_desc,
- "org.gnu.gdb.mips.dsp");
- /* The DSP registers are optional; it's OK if they are absent. */
- if (feature != NULL)
- {
- i = 0;
- valid_p = 1;
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "hi1");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "lo1");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "hi2");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "lo2");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "hi3");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspacc + i++, "lo3");
-
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- dspctl, "dspctl");
-
- if (!valid_p)
- {
- tdesc_data_cleanup (tdesc_data);
- return NULL;
- }
-
- mips_regnum.dspacc = dspacc;
- mips_regnum.dspctl = dspctl;
-
- num_regs = mips_regnum.dspctl + 1;
- }
- }
-
- /* It would be nice to detect an attempt to use a 64-bit ABI
- when only 32-bit registers are provided. */
- reg_names = NULL;
- }
-
/* First of all, extract the elf_flags, if available. */
if (info.abfd && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
elf_flags = elf_elfheader (info.abfd)->e_flags;
@@ -8442,10 +8263,185 @@ mips_gdbarch_init (struct gdbarch_info i
&& tdesc_property (info.target_desc, PROPERTY_GP32) != NULL
&& mips_abi != MIPS_ABI_EABI32
&& mips_abi != MIPS_ABI_O32)
+ return NULL;
+
+ /* Fill in the OS dependent register numbers and names. */
+ if (info.osabi == GDB_OSABI_LINUX)
{
- if (tdesc_data != NULL)
- tdesc_data_cleanup (tdesc_data);
- return NULL;
+ mips_regnum.fp0 = 38;
+ mips_regnum.pc = 37;
+ mips_regnum.cause = 36;
+ mips_regnum.badvaddr = 35;
+ mips_regnum.hi = 34;
+ mips_regnum.lo = 33;
+ mips_regnum.fp_control_status = 70;
+ mips_regnum.fp_implementation_revision = 71;
+ mips_regnum.dspacc = -1;
+ mips_regnum.dspctl = -1;
+ dspacc = 72;
+ dspctl = 78;
+ num_regs = 90;
+ reg_names = mips_linux_reg_names;
+ }
+ else
+ {
+ mips_regnum.lo = MIPS_EMBED_LO_REGNUM;
+ mips_regnum.hi = MIPS_EMBED_HI_REGNUM;
+ mips_regnum.badvaddr = MIPS_EMBED_BADVADDR_REGNUM;
+ mips_regnum.cause = MIPS_EMBED_CAUSE_REGNUM;
+ mips_regnum.pc = MIPS_EMBED_PC_REGNUM;
+ mips_regnum.fp0 = MIPS_EMBED_FP0_REGNUM;
+ mips_regnum.fp_control_status = 70;
+ mips_regnum.fp_implementation_revision = 71;
+ mips_regnum.dspacc = dspacc = -1;
+ mips_regnum.dspctl = dspctl = -1;
+ num_regs = MIPS_LAST_EMBED_REGNUM + 1;
+ if (info.bfd_arch_info != NULL
+ && info.bfd_arch_info->mach == bfd_mach_mips3900)
+ reg_names = mips_tx39_reg_names;
+ else
+ reg_names = mips_generic_reg_names;
+ }
+
+ /* Check any target description for validity. */
+ if (tdesc_has_registers (info.target_desc))
+ {
+ static const char *const mips_gprs[] = {
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+ "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+ "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"
+ };
+ static const char *const mips_fprs[] = {
+ "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
+ "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
+ "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
+ "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
+ };
+
+ const struct tdesc_feature *feature;
+ int valid_p;
+
+ feature = tdesc_find_feature (info.target_desc,
+ "org.gnu.gdb.mips.cpu");
+ if (feature == NULL)
+ return NULL;
+
+ tdesc_data = tdesc_data_alloc ();
+
+ valid_p = 1;
+ for (i = MIPS_ZERO_REGNUM; i <= MIPS_RA_REGNUM; i++)
+ valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
+ mips_gprs[i]);
+
+
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.lo, "lo");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.hi, "hi");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.pc, "pc");
+
+ if (!valid_p)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ feature = tdesc_find_feature (info.target_desc,
+ "org.gnu.gdb.mips.cp0");
+ if (feature == NULL)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ valid_p = 1;
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.badvaddr, "badvaddr");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ MIPS_PS_REGNUM, "status");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.cause, "cause");
+
+ if (!valid_p)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ /* FIXME drow/2007-05-17: The FPU should be optional. The MIPS
+ backend is not prepared for that, though. */
+ feature = tdesc_find_feature (info.target_desc,
+ "org.gnu.gdb.mips.fpu");
+ if (feature == NULL)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ valid_p = 1;
+ for (i = 0; i < 32; i++)
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ i + mips_regnum.fp0, mips_fprs[i]);
+
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.fp_control_status,
+ "fcsr");
+ valid_p
+ &= tdesc_numbered_register (feature, tdesc_data,
+ mips_regnum.fp_implementation_revision,
+ "fir");
+
+ if (!valid_p)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ num_regs = mips_regnum.fp_implementation_revision + 1;
+
+ if (dspacc >= 0)
+ {
+ feature = tdesc_find_feature (info.target_desc,
+ "org.gnu.gdb.mips.dsp");
+ /* The DSP registers are optional; it's OK if they are absent. */
+ if (feature != NULL)
+ {
+ i = 0;
+ valid_p = 1;
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "hi1");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "lo1");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "hi2");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "lo2");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "hi3");
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspacc + i++, "lo3");
+
+ valid_p &= tdesc_numbered_register (feature, tdesc_data,
+ dspctl, "dspctl");
+
+ if (!valid_p)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+
+ mips_regnum.dspacc = dspacc;
+ mips_regnum.dspctl = dspctl;
+
+ num_regs = mips_regnum.dspctl + 1;
+ }
+ }
+
+ /* It would be nice to detect an attempt to use a 64-bit ABI
+ when only 32-bit registers are provided. */
+ reg_names = NULL;
}
/* Try to find a pre-existing architecture. */
^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed 2/2] MIPS: Don't use a 32-bit BFD architecture with a 64-bit ABI
2018-02-26 19:48 [committed 0/2] MIPS: Don't use a 32-bit architecture with a 64-bit ABI Maciej W. Rozycki
2018-02-26 19:49 ` [committed 1/2] MIPS: Reorder ABI determination ahead of target description loading Maciej W. Rozycki
@ 2018-02-26 19:50 ` Maciej W. Rozycki
1 sibling, 0 replies; 3+ messages in thread
From: Maciej W. Rozycki @ 2018-02-26 19:50 UTC (permalink / raw)
To: gdb-patches
Select `bfd_mach_mips4000', which corresponds to the MIPS III ISA, the
earlies with 64-bit support, whenever a 32-bit BFD architecture has been
chosen to use with a 64-bit ABI. The situation can happen in a few
cases:
1. When the user has used `set architecture' or `set mips abi' commands
to override automatic selection and then starts a debug session by
requesting to run, attach or connect to a target.
2. In native debugging when reattaching to a previously debugged process
where the program to be debugged has been since discarded, as
observed with:
FAIL: gdb.base/attach.exp: attach2, with no file (GDB internal error)
in n32 and n64 regression testing.
3. In remote debugging with a non-XML debug stub when discarding the
program to be debugged while connected to the remote target, as
observed with:
FAIL: gdb.base/break-unload-file.exp: cmdline: always-inserted on: break: file (GDB internal error)
in n32 and n64 regression testing.
In the latter two cases the ABI, quite rightfully, is retained while the
program to be debugged is discarded. This is because in that case the
ABI previously determined is carried over along with `gdbarch' in use,
which is retained. The BFD architecture is however discarded and the
default then applies, because it is not attached to `gdbarch'.
In all these cases we trip with an internal error message as follows:
.../gdb/mips-tdep.c:766: internal-error: bad register size
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) n
This is a bug, please report it. For instructions, see:
<http://www.gnu.org/software/gdb/bugs/>.
coming from `mips_pseudo_register_read', because the raw register width
inferred from the BFD architecture turns out to be 4 for the general
registers while the cooked register width inferred from the ABI in
effect is 8.
We do not hit this internal error in remote debugging with an XML debug
stub, because in that case raw register width information is passed by
the stub along with the XML target description.
Ultimately I think we ought to make the BFD architecture sticky like the
ABI, however in the interim this simple fix will do, removing the error
across all three cases. The case where the user has used `set mips abi'
or `set architecture' commands has to be handled anyway, and although a
more sophisticated solution could be envisaged, such as reporting an
error with the respective `set' command, I think this is too much of a
corner case to bother.
gdb/
* mips-tdep.c (mips_gdbarch_init): Don't use a 32-bit BFD
architecture with a 64-bit ABI.
---
gdb/mips-tdep.c | 8 ++++++++
1 file changed, 8 insertions(+)
gdb-mips-gdbarch-init-arches-isa-from-abi.diff
Index: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c 2017-12-14 10:32:34.439700558 +0000
+++ binutils/gdb/mips-tdep.c 2017-12-14 10:34:14.003034347 +0000
@@ -8185,6 +8185,14 @@ mips_gdbarch_init (struct gdbarch_info i
fprintf_unfiltered (gdb_stdlog, "mips_gdbarch_init: mips_abi = %d\n",
mips_abi);
+ /* Make sure we don't use a 32-bit architecture with a 64-bit ABI. */
+ if (mips_abi != MIPS_ABI_EABI32
+ && mips_abi != MIPS_ABI_O32
+ && info.bfd_arch_info != NULL
+ && info.bfd_arch_info->arch == bfd_arch_mips
+ && info.bfd_arch_info->bits_per_word < 64)
+ info.bfd_arch_info = bfd_lookup_arch (bfd_arch_mips, bfd_mach_mips4000);
+
/* Determine the default compressed ISA. */
if ((elf_flags & EF_MIPS_ARCH_ASE_MICROMIPS) != 0
&& (elf_flags & EF_MIPS_ARCH_ASE_M16) == 0)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-26 19:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-26 19:48 [committed 0/2] MIPS: Don't use a 32-bit architecture with a 64-bit ABI Maciej W. Rozycki
2018-02-26 19:49 ` [committed 1/2] MIPS: Reorder ABI determination ahead of target description loading Maciej W. Rozycki
2018-02-26 19:50 ` [committed 2/2] MIPS: Don't use a 32-bit BFD architecture with a 64-bit ABI 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).