From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Luis Machado <luis.machado@arm.com>
Cc: John Baldwin <jhb@FreeBSD.org>, Tom Tromey <tom@tromey.com>,
gdb-patches@sourceware.org
Subject: Re: [PATCH 0/7] Fix race in DWARF reader
Date: Wed, 10 Apr 2024 01:02:27 +0100 (BST) [thread overview]
Message-ID: <alpine.DEB.2.21.2404092341230.53456@angie.orcam.me.uk> (raw)
In-Reply-To: <8114d4f1-a2ed-4f6f-aa64-339cfde9914b@arm.com>
On Tue, 9 Apr 2024, Luis Machado wrote:
> >> Tom> The main patch affects MIPS16. I can't test this -- I tried on a MIPS
> >> Tom> machine in the GCC compile farm, but unfortunately the relevant
> >> Tom> gdb.arch test says that the processor doesn't support MIPS16. It's
> >> Tom> possible this code is simply dead; I do not know.
> >>
> >> Elsewhere I mentioned that I had a different idea for this series.
> >>
> >> It seems to me that most (or maybe even all) the calls to
> >> dwarf2_per_objfile::adjust aren't really needed. Many of them only
> >> affect lookup tables, where the adjustment isn't needed. This includes
> >> all calls made by the indexer.
> >>
> >> Some of the calls (like the one in read_attribute_value) even seem to be
> >> wrong.
> >>
> >> So, I wrote a short series to remove these. Unfortunately, though, it's
> >> hard to know for sure if the result is correct, given that I don't know
> >> how to test MIPS16.
> >>
> >> I could probably test some simple things ("break") by debugging gdb
> >> while examining (but not running) a MIPS16 program. I'm not sure if
> >> that's really sufficient though.
> >>
> >> I'd appreciate some insight if you have any.
> >
> > I haven't seen anyone active with submitting MIPS patches in several years.
> > I no longer make use of MIPS myself (and we've removed it from FreeBSD
> > entirely, though I know it's still present in Linux distros). Even when I
> > was working with MIPS I never tested microMIPS / MIPS16.
Support for the MIPS target in Linux is certainly far from being dead and
I believe new MIPS hardware continues being made. Also a substantial MIPS
patch for GDB for R6 ISA support is being pinged for review right now.
> > OTOH, I think MIPS16 is similar to Thumb on ARM, and it might even be
> > using a similar trick from reading your series (setting the LSB to enter
> > "compressed decoding mode" vs "regular decoding mode"). I think ARM uses
> > special mapping symbols to mark Thumb vs non-Thumb code though instead of
> > depending on the LSB? That is, I wonder why Thumb doesn't trip over this
> > issue the way MIPS16 does?
> >
>
> cc-ing Maciej, who might have a better idea on MIPS bits.
Thanks, Luis!
I can certainly run MIPS16 GDB verification right away with actual
hardware:
macro@malta(1)~$ uname -a
Linux malta 5.18.0-rc2-00254-gfb649bda6f56-dirty #2 Sat Nov 12 20:14:53 GMT 2022 mips unknown unknown GNU/Linux
macro@malta(2)~$ grep mips16 /proc/cpuinfo
ASEs implemented : mips16 dsp dsp2
macro@malta(3)~$
however to understand the impact I'd have to go through the code changes,
which I can't guarantee any specific timeframe for.
Indeed at the machine level it is the LSB of the PC that tells compressed
and regular code apart: you just flip the bit as required either by using
special instructions with direct calls/jumps or explicitly with indirect
ones; it's also correctly set according to the execution mode in effect in
PC values recorded by hardware, such as the return address for function
calls or the exception PC for kernel traps.
Then whether compressed code uses the MIPS16 instruction encoding or the
microMIPS instruction encoding it is the property of the implementation.
Offhand I recall for compressed functions DWARF line information has the
LSB of the PC set according to compressed vs regular encoding, however
other DWARF records or the static ELF symbol table do not. Compressed
function symbols have appropriate flags set in `st_other' to tell them
apart from regular function symbols. BFD uses that information to set the
LSB appropriately in relocation processing where applicable.
To call a compressed function by hand or for function pointer comparison
(e.g. against a datum stored in a program's variable or in a CPU register)
in expression evaluation GDB has to recreate the LSB from information
available and apply it to symbol values obtained from the symbol table,
and it's a bit messy due to how things happened in the past. Conversely,
in certain contexts the LSB has to be removed instead, such as `x /i $pc'.
I made all this at least work at one point, not without shortcomings (e.g.
broken hex instruction dumps in `disassemble /r' output), which is what we
have now.
Later on Yao Qi came up with a better proposal building on a generalised
property of some psABIs where a function pointer is not the function's
address: <https://sourceware.org/ml/gdb-patches/2016-10/msg00430.html>.
The proposal got stuck on an issue with the PPC64 target which got never
resolved due to the shortage of time and higher priority tasks combined:
<https://sourceware.org/ml/gdb-patches/2017-10/msg00096.html>.
Maybe someone can pick it up from there? I could do the necessary MIPS
bits then myself, I certainly find it important enough to preempt other
stuff I might otherwise want doing instead.
Maciej
next prev parent reply other threads:[~2024-04-10 0:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-18 1:10 Tom Tromey
2024-02-18 1:10 ` [PATCH 1/7] Compare section index in lookup_minimal_symbol_by_pc_section Tom Tromey
2024-02-18 1:10 ` [PATCH 2/7] Remove unnecessary null check " Tom Tromey
2024-02-18 1:10 ` [PATCH 3/7] Hoist a call to frob_address Tom Tromey
2024-02-18 1:10 ` [PATCH 4/7] Add unrelocated overload of lookup_minimal_symbol_by_pc_section Tom Tromey
2024-02-18 1:10 ` [PATCH 5/7] Fix race in background DWARF indexer Tom Tromey
2024-02-18 1:10 ` [PATCH 6/7] Use bound_minimal_symbol more in lookup_minimal_symbol_by_pc_section Tom Tromey
2024-02-18 1:10 ` [PATCH 7/7] Fix address comparison " Tom Tromey
2024-04-03 15:16 ` [PATCH 0/7] Fix race in DWARF reader Tom Tromey
2024-04-09 18:16 ` John Baldwin
2024-04-09 22:11 ` Luis Machado
2024-04-10 0:02 ` Maciej W. Rozycki [this message]
2024-04-16 17:05 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2404092341230.53456@angie.orcam.me.uk \
--to=macro@orcam.me.uk \
--cc=gdb-patches@sourceware.org \
--cc=jhb@FreeBSD.org \
--cc=luis.machado@arm.com \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).