RE: [PATCH 1/3] gdb, amd64: extend the amd64 prologue analyzer to skip register pushes

["Kupczak, Pawel" <pawel.kupczak@intel.com>]

> -----Original Message-----
> From: Kupczak, Pawel <pawel.kupczak@intel.com>
> Sent: Friday, July 18, 2025 5:15 PM
> To: Guinevere Larsen <guinevere@redhat.com>; gdb-
> patches@sourceware.org
> Subject: RE: [PATCH 1/3] gdb, amd64: extend the amd64 prologue analyzer to
> skip register pushes
> 
> Hi!  Thanks for taking a look.
> 
> > -----Original Message-----
> > From: Guinevere Larsen <guinevere@redhat.com>
> > Sent: Friday, July 18, 2025 3:44 PM
> > To: Kupczak, Pawel <pawel.kupczak@intel.com>; gdb-
> > patches@sourceware.org
> > Subject: Re: [PATCH 1/3] gdb, amd64: extend the amd64 prologue analyzer
> to
> > skip register pushes
> >
> > Hello! Thanks for working on this!
> >
> > I am not an authority on i386, but I did notice a few things that gave
> > me pause so I'd like to ask them to make sure I understand the changes.
> >
> > Skipping the commit message:
> > On 7/1/25 7:47 AM, Pawel Kupczak wrote:
> > > ---
> > >   gdb/amd64-tdep.c                              | 53 +++++++++++-
> > >   .../amd64-extended-prologue-analysis.c        | 49 +++++++++++
> > >   .../amd64-extended-prologue-analysis.exp      | 86
> > +++++++++++++++++++
> > >   3 files changed, 187 insertions(+), 1 deletion(-)
> > >   mode change 100644 => 100755 gdb/amd64-tdep.c
> > >   create mode 100644 gdb/testsuite/gdb.arch/amd64-extended-prologue-
> > analysis.c
> > >   create mode 100644 gdb/testsuite/gdb.arch/amd64-extended-prologue-
> > analysis.exp
> > >
> > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> > > old mode 100644
> > > new mode 100755
> > > index 82dd1e07cf3..863b29a8c27
> > > --- a/gdb/amd64-tdep.c
> > > +++ b/gdb/amd64-tdep.c
> > > @@ -2553,6 +2553,56 @@ amd64_analyze_frame_setup (gdbarch
> > *gdbarch, CORE_ADDR pc,
> > >     return pc;
> > >   }
> > >
> > > +/* Check whether PC points at code pushing registers onto the stack.  If
> so,
> > > +   update CACHE and return pc after those pushes or CURRENT_PC,
> > whichever is
> > > +   smaller.  Otherwise, return PC passed to this function.  */
> > > +
> > > +static CORE_ADDR
> > > +amd64_analyze_register_saves (CORE_ADDR pc, CORE_ADDR current_pc,
> > > +			      amd64_frame_cache *cache)
> > > +{
> > > +  gdb_byte op;
> > > +  int offset = 0;
> > > +
> > > +  /* There are at most 16 registers that would be pushed in the prologue.
> */
> > > +  for (int i = 0; i < 16 && pc < current_pc; i++)
> > > +    {
> > > +      int reg = 0;
> > > +      int pc_offset = 0;
> > > +
> > > +      if (target_read_code (pc, &op, 1) == -1)
> > > +	return pc;
> > > +
> > > +      /* %r8 - %r15 prefix.  */
> > > +      if (op == 0x41)
> >
> > Looking over on the disassembler for record-full, I see that all 0x4-
> > are considered prefixes, and (in 64 bit targets) they all have this effect.
> >
> > Is this something that could affect this prologue analyzer? ie, that
> > some prefix like 0x40 is used to mean "push a register larger than 7",
> > which would cause us to not skip that instruction?
> >
> 
> Now that I checked AMD64 manual, it indeed mentions that this prefix
> ranges from 0x40 - 0x4F.  I might've tunneled too hard on one value, I
> checked compiler explorer now and it uses a different one too (0x48).

Correction on that - first, that 0x48 never actually happened (I was looking
at the wrong thing).

Second, regarding REX values.  I think for those register pushes, 0x41 is the
only value relevant here.  Looking at instruction set and reading about the
prefix at https://dn720004.ca.archive.org/0/items/os-dev-manuals/amd%20amd64%20sdm%20instructions.pdf,
it mentions in section 1.4.4 the meanings of the 4 bits.  I think in this case
REX.B is the only one that's gonna be set:
'push' will always take 64 bit operand here so REX.W would not be set.
'push' also doesn't use ModRM AFAIK so REX.R shouldn't be applicable.
REX.X seems completely irrelevant.

Based on that I think REX.B is the only bit that will be set in such cases,
which maps to REX == 0x41. 

Let me know your thoughts :D

With regards,
Paweł
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.