[v8,00/10] Extensions for PTWRITE

  Eli already approved the documentation parts.
Markus reviewed every patch and approved the btrace bits.
The only thing that is missing is a maintainers approval for the Python
bits.

v7 can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-October/192830.html

I pinged this version 12 times, but didn't get any response.
Since the last time I posted this was half a year ago, I decided to re-post a
rebased version again.  I only had to solve some minor conflicts.  The
biggest change was using the new require logic for the testing.

Thanks,
Felix

Felix Willgerodt (10):
  btrace: Introduce auxiliary instructions.
  btrace: Enable auxiliary instructions in record instruction-history.
  btrace: Enable auxiliary instructions in record function-call-history.
  btrace: Handle stepping and goto for auxiliary instructions.
  python: Introduce gdb.RecordAuxiliary class.
  python: Add clear() to gdb.Record.
  btrace, gdbserver: Add ptwrite to btrace_config_pt.
  btrace, linux: Enable ptwrite packets.
  btrace, python: Enable ptwrite filter registration.
  btrace: Extend ptwrite event decoding.

 gdb/NEWS                                      |   7 +
 gdb/btrace.c                                  |  68 ++-
 gdb/btrace.h                                  |  38 +-
 gdb/config.in                                 |   3 +
 gdb/configure                                 |  11 +
 gdb/data-directory/Makefile.in                |   1 +
 gdb/disasm-flags.h                            |   1 +
 gdb/doc/gdb.texinfo                           |  32 +-
 gdb/doc/python.texi                           | 168 ++++++
 gdb/extension-priv.h                          |   5 +
 gdb/extension.c                               |  13 +
 gdb/extension.h                               |   3 +
 gdb/features/btrace-conf.dtd                  |   1 +
 gdb/guile/guile.c                             |   1 +
 gdb/nat/linux-btrace.c                        |  29 +
 gdb/python/lib/gdb/ptwrite.py                 |  80 +++
 gdb/python/py-record-btrace.c                 | 136 ++++-
 gdb/python/py-record-btrace.h                 |  11 +
 gdb/python/py-record.c                        |  89 ++-
 gdb/python/py-record.h                        |   3 +
 gdb/python/python-internal.h                  |   3 +
 gdb/python/python.c                           |   2 +
 gdb/record-btrace.c                           | 107 +++-
 gdb/record.c                                  |  10 +
 gdb/record.h                                  |   5 +-
 gdb/remote.c                                  |  33 ++
 gdb/testsuite/gdb.btrace/i386-ptwrite.S       | 550 ++++++++++++++++++
 gdb/testsuite/gdb.btrace/ptwrite.c            |  39 ++
 gdb/testsuite/gdb.btrace/ptwrite.exp          | 200 +++++++
 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S     | 544 +++++++++++++++++
 gdb/testsuite/gdb.python/py-record-btrace.exp |   6 +-
 gdb/testsuite/lib/gdb.exp                     |  72 +++
 gdbserver/linux-low.cc                        |   3 +
 gdbserver/server.cc                           |  18 +
 gdbsupport/btrace-common.h                    |   6 +
 gdbsupport/common.m4                          |   2 +
 gdbsupport/config.in                          |   3 +
 gdbsupport/configure                          |  11 +
 38 files changed, 2279 insertions(+), 35 deletions(-)
 create mode 100644 gdb/python/lib/gdb/ptwrite.py
 create mode 100644 gdb/testsuite/gdb.btrace/i386-ptwrite.S
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.c
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.exp
 create mode 100644 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S
  

On 3/21/23 11:46, Felix Willgerodt via Gdb-patches wrote:
> Eli already approved the documentation parts.
> Markus reviewed every patch and approved the btrace bits.
> The only thing that is missing is a maintainers approval for the Python
> bits.
> 
> v7 can be found here:
> https://sourceware.org/pipermail/gdb-patches/2022-October/192830.html
> 
> I pinged this version 12 times, but didn't get any response.
> Since the last time I posted this was half a year ago, I decided to re-post a
> rebased version again.  I only had to solve some minor conflicts.  The
> biggest change was using the new require logic for the testing.

Hi Felix,

Sorry for that.  With the amount of stuff on the list, it's easy to
overlook things and think "ehhh, I'm not comfortable reviewing this,
I'll let someone else do it".  I will check the Python bits.

git shows these little things to fix when applying the series:

    ➜  binutils-gdb git:(master) ✗ git am ./v8_20230321_felix_willgerodt_extensions_for_ptwrite.mbx
    Applying: btrace: Introduce auxiliary instructions.
    Applying: btrace: Enable auxiliary instructions in record instruction-history.
    Applying: btrace: Enable auxiliary instructions in record function-call-history.
    Applying: btrace: Handle stepping and goto for auxiliary instructions.
    .git/rebase-apply/patch:29: indent with spaces.
            {
    .git/rebase-apply/patch:30: indent with spaces.
              *replay = start;
    .git/rebase-apply/patch:31: indent with spaces.
              return btrace_step_no_history ();
    .git/rebase-apply/patch:32: indent with spaces.
            }
    warning: 4 lines add whitespace errors.
    Applying: python: Introduce gdb.RecordAuxiliary class.
    Applying: python: Add clear() to gdb.Record.
    .git/rebase-apply/patch:106: trailing whitespace.
        gdb_test "python print(i.is_speculative)" "False"
    warning: 1 line adds whitespace errors.
    Applying: btrace, gdbserver: Add ptwrite to btrace_config_pt.
    Applying: btrace, linux: Enable ptwrite packets.
    Applying: btrace, python: Enable ptwrite filter registration.
    Applying: btrace: Extend ptwrite event decoding.
    .git/rebase-apply/patch:285: trailing whitespace.
    (gdb) info threads
    .git/rebase-apply/patch:1187: trailing whitespace.
    # SUCC: EXIT [always]
    .git/rebase-apply/patch:1227: trailing whitespace.
    # SUCC: EXIT [always]
    .git/rebase-apply/patch:1261: trailing whitespace.
    # SUCC: EXIT [always]
    warning: 4 lines add whitespace errors.

Simon
  

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Sorry for that.  With the amount of stuff on the list, it's easy to
Simon> overlook things and think "ehhh, I'm not comfortable reviewing this,
Simon> I'll let someone else do it".

This is exactly what I did as well.  I see it as a kind of collective
action problem.

It's clear we need a new system here, but I don't know what it is.  I
was hoping patchworks, but that turned out to require too much manual
intervention, IMO.

Maybe we should simply go back to gerrit, despite its issues.  Perhaps
we could do it in conjunction with a robot that auto-assigns reviews.

Another option that would work with the above is to add more reviewers.
However, I don't think that just doing this would be enough -- there is
a structural problem here that we should address.

Tom
  

On 3/24/23 14:23, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Sorry for that.  With the amount of stuff on the list, it's easy to
> Simon> overlook things and think "ehhh, I'm not comfortable reviewing this,
> Simon> I'll let someone else do it".
> 
> This is exactly what I did as well.  I see it as a kind of collective
> action problem.
> 
> It's clear we need a new system here, but I don't know what it is.  I
> was hoping patchworks, but that turned out to require too much manual
> intervention, IMO.
> 
> Maybe we should simply go back to gerrit, despite its issues.  Perhaps
> we could do it in conjunction with a robot that auto-assigns reviews.

I don't think just Gerrit would have helped, I would have ignored it on
Gerrit as well.  Perhaps some robot, as you said, would have helped, but
I'm not sure it's good to just randomly assign reviews.  We all have our
areas of knowledge, and I think it's good to respect that.

Perhaps some interactive patch triage / review session like glibc has
would help, but I'm not sure people (including me) want to commit to a
regular meeting for that.

Simon
  

Simon> I don't think just Gerrit would have helped, I would have ignored it on
Simon> Gerrit as well.  Perhaps some robot, as you said, would have helped, but
Simon> I'm not sure it's good to just randomly assign reviews.  We all have our
Simon> areas of knowledge, and I think it's good to respect that.

What Rust did is have a robot auto-assign, but then the assignee could
reassign the review to someone else if they thought they wouldn't do it
well.

The idea is to avoid the situation where no one in particular is
responsible for a given patch.

Tom
  

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Freitag, 24. März 2023 14:57
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v8 00/10] Extensions for PTWRITE
> 
> On 3/21/23 11:46, Felix Willgerodt via Gdb-patches wrote:
> > Eli already approved the documentation parts.
> > Markus reviewed every patch and approved the btrace bits.
> > The only thing that is missing is a maintainers approval for the Python
> > bits.
> >
> > v7 can be found here:
> > https://sourceware.org/pipermail/gdb-patches/2022-October/192830.html
> >
> > I pinged this version 12 times, but didn't get any response.
> > Since the last time I posted this was half a year ago, I decided to re-post a
> > rebased version again.  I only had to solve some minor conflicts.  The
> > biggest change was using the new require logic for the testing.
> 
> Hi Felix,
> 
> Sorry for that.  With the amount of stuff on the list, it's easy to
> overlook things and think "ehhh, I'm not comfortable reviewing this,
> I'll let someone else do it".  I will check the Python bits.

No worries, thanks for doing that!

> git shows these little things to fix when applying the series:
> 
>     ➜  binutils-gdb git:(master) ✗ git am
> ./v8_20230321_felix_willgerodt_extensions_for_ptwrite.mbx
>     Applying: btrace: Introduce auxiliary instructions.
>     Applying: btrace: Enable auxiliary instructions in record instruction-history.
>     Applying: btrace: Enable auxiliary instructions in record function-call-
> history.
>     Applying: btrace: Handle stepping and goto for auxiliary instructions.
>     .git/rebase-apply/patch:29: indent with spaces.
>             {
>     .git/rebase-apply/patch:30: indent with spaces.
>               *replay = start;
>     .git/rebase-apply/patch:31: indent with spaces.
>               return btrace_step_no_history ();
>     .git/rebase-apply/patch:32: indent with spaces.
>             }
>     warning: 4 lines add whitespace errors.
>     Applying: python: Introduce gdb.RecordAuxiliary class.
>     Applying: python: Add clear() to gdb.Record.
>     .git/rebase-apply/patch:106: trailing whitespace.
>         gdb_test "python print(i.is_speculative)" "False"
>     warning: 1 line adds whitespace errors.
>     Applying: btrace, gdbserver: Add ptwrite to btrace_config_pt.
>     Applying: btrace, linux: Enable ptwrite packets.
>     Applying: btrace, python: Enable ptwrite filter registration.
>     Applying: btrace: Extend ptwrite event decoding.
>     .git/rebase-apply/patch:285: trailing whitespace.
>     (gdb) info threads
>     .git/rebase-apply/patch:1187: trailing whitespace.
>     # SUCC: EXIT [always]
>     .git/rebase-apply/patch:1227: trailing whitespace.
>     # SUCC: EXIT [always]
>     .git/rebase-apply/patch:1261: trailing whitespace.
>     # SUCC: EXIT [always]
>     warning: 4 lines add whitespace errors.
> 
> Simon

Argh, thanks for pointing those out. I will fix them.

Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928