[Bug testsuite/31484] New: [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[Bug testsuite/31484] New: [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

            Bug ID: 31484
           Summary: [gdb/testsuite] Add memcpy, memmove, memset watchpoint
                    tests
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: testsuite
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

While discussing aarch64 watchpoints, the point was raised that it's not clear
what addr_trap the kernel will report when using the aarch64 mops feature ops (
CPYF*, CPY*, SET* and SETG*).

For now, there doesn't seem to a an SOC out there supporting this (looking at
https://gpages.juszkiewicz.com.pl/arm-socs-table/arm-socs.html).

Nevertheless, we can add test-cases that excercise memset/memcpy/memmove and
set watchpoints.  On a mops architecture, using those will active the optimized
versions in glibc which will use the mops feature ops.

Likewise, on other architecture optimized versions may be triggered which favor
larger memory access operations, which are interesting for testing watchpoint
behaviour.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |luis.machado at arm dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #1 from Luis Machado <luis.machado at arm dot com> ---
QEMU already supports "FEAT_MOPS (Standardization of memory operations)", so it
could be used for early experimentation of what we may see in actual hardware,
and to help craft a testcase.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

Thiago Jung Bauermann <thiago.bauermann at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thiago.bauermann at linaro dot org

--- Comment #2 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
I am working on this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[brobecker at gnat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

Joel Brobecker <brobecker at gnat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |brobecker at gnat dot com

--- Comment #3 from Joel Brobecker <brobecker at gnat dot com> ---
Hello,

What do you guys think of renaming this PR to implementing the missing
features, and then adding the testcases can be part of the implementation?

That way, the implementation of the features can be tracked via this PR.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Joel Brobecker from comment #3)
> Hello,
> 
> What do you guys think of renaming this PR to implementing the missing
> features, and then adding the testcases can be part of the implementation?
> 
> That way, the implementation of the features can be tracked via this PR.

I filed this as a testsuite PR, with the idea that implementing the
test-case(s) might enable us to find missing support in gdb.

As of yet, I'm not sure if there is anything missing, so renaming doesn't make
sense to me, but if it does to someone else I don't object.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #5 from Luis Machado <luis.machado at arm dot com> ---
I'm fine with renaming if it makes it easier on planning/organization of the
release.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #6 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
Hello,

Sorry for the delay in reporting back here. I agree with Tom that it makes
sense to keep this PR for the testcases, as they aren't really AArch64-specific
and could uncover needed support in GDB for other architectures as well.

I created a very simple test program and ran it under GDB with glibc 2.39 —
which introduces the MOPS variants for memset, memcpy and memmove — on both
QEMU with -mcpu max and also on the Arm FVP emulator. GDB behaved pretty
normally regarding watchpoints triggered by memcpy, memmove and memset,
stopping where it's supposed to.

This is because both emulators report the exact address of the watchpoint as
the data trap address, so GDB has no problem identifying which watchpoint was
triggered. Real hardware is allowed by the spec to be less precise, so
unfortunately my experiment may not be representative of GDB behaviour there.

I'm finishing the last touches on the patch introducing the testcase, and will
post it on the mailing list tomorrow.

I'll open a separate PR for AArch64-specific MOPS support, because in the next
few days I'll send a couple of patches that are needed there: one for decoding
the new instructions for reverse execution, and one to treat a sequence of MOPS
instructions as atomic, to prevent GDB from breaking or stepping in the middle
of the sequence.

Does that sound OK?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[brobecker at gnat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #7 from Joel Brobecker <brobecker at gnat dot com> ---
> Does that sound OK?

Sounds good to me, Thiago. If you wouldn't mind, can you post here the URL of
the new PR? And it would be useful also in the other PR if you posted the URL
of the patch series once posted on gdb-patches. I try to locate them through
the PR number, but sometimes I miss things in my searches...

Thank you!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #8 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
(In reply to Joel Brobecker from comment #7)
> > Does that sound OK?
> 
> Sounds good to me, Thiago. If you wouldn't mind, can you post here the URL
> of the new PR?

Thank you! Here is the new PR for the AArch64-specific patches:

https://sourceware.org/bugzilla/show_bug.cgi?id=31666

> And it would be useful also in the other PR if you posted the URL of the
> patch series once posted on gdb-patches. I try to locate them through the
> PR number, but sometimes I miss things in my searches...

Of course. I'll post the patches ASAP and put the URLs in the PR. Sorry for
being late with this. I appreciate the patience.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #9 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
(In reply to Thiago Jung Bauermann from comment #6)
> I'm finishing the last touches on the patch introducing the testcase, and
> will post it on the mailing list tomorrow.

Here is v2 of the testcase:

https://inbox.sourceware.org/gdb-patches/20240421222657.1052635-1-thiago.bauermann@linaro.org/

It found bug #31665 on 32-bit arm, so we must be on to something :-)

Unfortunately I posted these patches without seeing Kevin's review of v1, so
I'll need to send a v3.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #10 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
Just sent v3:

https://inbox.sourceware.org/gdb-patches/20240422230700.1173173-1-thiago.bauermann@linaro.org/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #11 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thiago Bauermann
<bauermann@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6b4b601b35a0aaf2b8b5db8e5793302736504171

commit 6b4b601b35a0aaf2b8b5db8e5793302736504171
Author: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Date:   Thu Apr 18 23:24:55 2024 -0300

    gdb/testsuite: Add gdb.base/memops-watchpoint.exp

    Test behaviour of watchpoints triggered by libc's memset/memcpy/memmove.
    These functions are frequently optimized with specialized instructions
    that favor larger memory access operations, so make sure GDB behaves
    correctly in their presence.

    There's a separate watched variable for each function so that the testcase
    can test whether GDB correctly identified the watchpoint that triggered.

    Also, the watchpoint is 28 bytes away from the beginning of the buffer
    being modified, so that large memory accesses (if present) are exercised.

    PR testsuite/31484
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31484

    Approved-by: Kevin Buettner <kevinb@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[thiago.bauermann at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #12 from Thiago Jung Bauermann <thiago.bauermann at linaro dot org> ---
(In reply to Sourceware Commits from comment #11)
> The master branch has been updated by Thiago Bauermann
> <bauermann@sourceware.org>:
> 
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;
> h=6b4b601b35a0aaf2b8b5db8e5793302736504171
> 
> commit 6b4b601b35a0aaf2b8b5db8e5793302736504171
> Author: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Date:   Thu Apr 18 23:24:55 2024 -0300
> 
>     gdb/testsuite: Add gdb.base/memops-watchpoint.exp

Thanks to Kevin, I believe this bugzilla is now done. But if Tom or Luis have
any any additional suggestions, I'd be happy to extend or modify the testcase.

There's still the AArch64-specific #31666, for which I'm getting asymptotically
close to posting patches.

>     Approved-by: Kevin Buettner <kevinb@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

--- Comment #13 from Luis Machado <luis.machado at arm dot com> ---
Overall looks like good improvements.

Most of the issues seem to arise from attempting odd combinations of memory
access size and alignments. I think a few of these cases are covered by
gdb.base/watchpoint-unaligned.exp, but limited to a small number of
alignments/memory access sizes.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31484] [gdb/testsuite] Add memcpy, memmove, memset watchpoint tests

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31484

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |15.1
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #14 from Tom de Vries <vries at gcc dot gnu.org> ---
Thanks Thiago.

The test-case looks like what I wanted to have when I wrote the description.

Marking resolved-fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.