valgrind-developers — Discussion list for developers of valgrind

[Valgrind-developers] [valgrind] Fix a panic message.

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=d2fea8d23e85ed573094f6d945b224f3e730e90c

commit d2fea8d23e85ed573094f6d945b224f3e730e90c
Author: Florian Krohm <fl...@ei...>
Date:   Wed Jul 30 14:44:40 2025 +0000

    Fix a panic message.
    
    That code snippet is executed for both Iend_BE and Iend_LE.

Diff:
---
 memcheck/mc_translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c
index 05e6d59afa..b4e499cc00 100644
--- a/memcheck/mc_translate.c
+++ b/memcheck/mc_translate.c
@@ -6064,7 +6064,7 @@ void do_shadow_Store ( MCEnv* mce,
          case Ity_I32:  c = IRConst_U32 (V_BITS32_DEFINED); break;
          case Ity_I16:  c = IRConst_U16 (V_BITS16_DEFINED); break;
          case Ity_I8:   c = IRConst_U8  (V_BITS8_DEFINED);  break;
-         default:       VG_(tool_panic)("memcheck:do_shadow_Store(LE)");
+         default:       VG_(tool_panic)("memcheck:do_shadow_Store");
       }
       vdata = IRExpr_Const( c );
    }

[Valgrind-developers] Remove U256 type from VEX

[Florian K. <fl...@ei...>]

U256 is a 16-byte wide type defined as an array of UInts and therefore 
presumably meant to represent 256 bit wide integers.
There are, however, no IROps operating on or yielding such a value.
The type is only used to declare guest_YMM0 etc. in libvex_guest_amd64.h
But those registers are vector registers. Therefore, V256 is the proper type.

The reg-tested attached patch removes the U256 typedef and adds a V128 [2] 
variant to V256.

I realise that this is an incompatible change in the VEX API.
But given that U256 is effectively a dead type within VEX I was wondering 
whether we have to go through some kind of deprecation protocol.

OK?

   Florian

[Valgrind-developers] Sourceware infrastructure updates for Q2 2025

[Mark W. <ma...@kl...>]

Sourceware infrastructure community updates for Q2 2025

Sourceware has provided the infrastructure for core toolchain
and developer tools projects for more than 25 years.
https://sourceware.org/sourceware-25-roadmap.html

  Keep Sourceware worry-free, friendly and independent by donating
  https://sourceware.org/donate.html support our fiscal sponser SFC
  https://sfconservancy.org/sustainer and/or support OSUOSL for
  hosting Free Software projects https://osuosl.org/donate/

Every quarter we provide a summary of news about Sourceware
infrastructure:

- Sourceware @ Conservancy Year Two
- Anubis for more services, now without javascript challenge
- Sourceware servers on the move
  - Red Hat Community Cage server move
  - OSUOSL datacenter move
- The Road to Porto: Cauldron in September
- Thanks Christopher Faylor (cgf)
- Signed-commit leaderboard
- Sourceware Organization, Contact and Open Office hours

= Sourceware @ Conservancy Year Two

  In May we celebrated that Sourceware got a financial and
  administrative home at the Software Freedom Conservancy two years
  ago https://sfconservancy.org/news/2023/may/15/sourceware-joins-sfc/

  Conservancy has helped us turn from a purely volunteer into a
  professional organization with an Project Leadership Committee,
  monthly open office hours, multiple hardware services partners,
  expanded services, and a more diverse funding model that allows us
  to enter into contracts with paid contractors or staff if necessary.

  Read all about the last year communications, user survey, the new
  services, cyber security and regulations, new and upgraded hardware,
  and our finances:
  https://inbox.sourceware.org/202...@gn...

= Anubis for more services, now without javascript challenge

  What started as an experiment for patchwork and bunsen has now been
  rolled out to most other services, cgit, gitweb, bugzilla, wikis and
  the forge.

  The latest version of Anubis contains a non-javascript challenge
  which has been used for bugzilla as an experiment and seems as
  effective as the javascript challenge. So it has now been rolled out
  to all services.

= Sourceware servers on the move

  All our servers will be moving later this year because both our
  hardware services partners will move datacenters. The Sourceware PLC
  decided to take advantage of this move by adding more/bigger
  machines.

  - Red Hat Community Cage server move

  This https://www.osci.io/tenants/ impacts server2 (main server),
  server3 (backup server) and forge.sourceware.org. We will add a new
  bigger server which has 3x memory [24x64GB], 10x storage [6x3.84TB],
  2x cpu ish [2x28 cores] compared to the current servers. The new
  data center also has a a faster/bigger network pipe.

  The new server1 was made possible thanks to the FUTO grant,
  individual Sourceware donations and Red Hat OSPO CommInfra & IT
  teams. It has already been installed in the new RDU3 data
  center. But doesn't have network yet (will be added in two weeks).

  We like to have the new server1 setup and in production before the
  move of the other two servers so there is a minimum of downtime. We
  discussed a plan to do this and how we can use this for moving some
  services in their own isolated VMs, and which resources need to be
  untangled for that at the last Open Office hour.

  https://sourceware.org/sourceware-wiki/Migration2025/
  
  - OSUOSL datacenter move

  This https://osuosl.org/communities/ impacts sourceware-builder1,
  sourceware-builder2, arm64-1, arm64-2 and the snapshots server. The
  OSL might be able to upgrade the first two CI x86_64 builders which
  would be great since we expect the experimental forge to also want
  to add CI for merge requests. But they would like us to cover some
  of the co-location hosting costs if possible.

= The Road to Porto: Cauldron in September

  The next GNU Tools Cauldron, taking place in Porto, Portugal, on
  September 26-28, 2025.

  https://inbox.sourceware.org/87o...@or.../
  https://gcc.gnu.org/wiki/cauldron2025
  https://gnu-tools-cauldron.org/

  Various Sourceware Project Leadership Committee members will be
  there to discuss various infrastructure projects and experiments.

= Signed-commit census leaderboard

  How is your project doing on signed commits?

  Analyzing branch HEAD since 2025-01-01
  cygwin-calm    32 commits  32 signed 100%   1 committers  1 signers 100%
  cygwin-setup   11 commits  11 signed 100%   1 committers  1 signers 100%
  gitsigur        5 commits   5 signed 100%   1 committers  1 signers 100%
  bunsen        163 commits 136 signed  83%   2 committers  2 signers 100%
  annobin        48 commits  39 signed  81%   2 committers  1 signers  50%
  systemtap      67 commits  50 signed  74%   4 committers  3 signers  75%
  builder        58 commits  18 signed  31%   4 committers  3 signers  75%
  elfutils      102 commits  14 signed  13%   4 committers  2 signers  50%
  lvm2          428 commits  41 signed   9%   7 committers  1 signers  14%
  glibc         741 commits  74 signed   9%  33 committers  2 signers   6%
  gcc          5659 commits 447 signed   7% 152 committers 10 signers   6%
  debugedit      15 commits   1 signed   6%   2 committers  1 signers  50%
  newlib-cygwin 416 commits  26 signed   6%  14 committers  2 signers  14%
  binutils-gdb 2244 commits  80 signed   3%  79 committers  5 signers   6%
  libabigail     81 commits   2 signed   2%   2 committers  1 signers  50%
  bzip2           2 commits   0 signed   0%   1 committers  0 signers   0%
  dwz             9 commits   0 signed   0%   2 committers  0 signers   0%
  insight        54 commits   0 signed   0%   1 committers  0 signers   0%
  forge          12 commits   0 signed   0%   1 committers  0 signers   0%
  valgrind      332 commits   0 signed   0%   6 committers  0 signers   0%

= Thanks Christopher Faylor (cgf)

  Since 1999 Christopher Faylor has been one of the Cygwin project
  leads for 15 years. He was list maintainer for cygwin, sourceware
  and gcc mailinglists for 20 years. And handled spam almost daily so
  we could have open lists. He was one of the founding members of the
  Sourceware Project Leadership Committee (PLC). Sourceware wouldn't
  be what it is today without him.

  But after 25 years of being involved with Sourceware and 2 years
  serving on the PLC he decided to resign. We thank him for all he did
  and all his insights making Sourceware a worry-free, friendly home
  for core toolchain and developer tools projects.

  The PLC https://sourceware.org/mission.html#plc now consists of 7
  members. The mandatory minimum number of Members is 4. And no more
  than 2 Members may be Financially-Related to the same Entity.

  If you are interested in joining the PLC please read the
  https://sourceware.org/Conservancy-Sourceware-FSA.pdf
  Fiscal Sponsorship Agreement, the Conflict of Interest Policy
  https://sfconservancy.org/projects/policies/conflict-of-interest-policy.html
  and contact us at pl...@so....

  If you rather help with more technical tasks please join the
  overseers list: https://sourceware.org/mailman/listinfo/overseers

= Sourceware Organization, Contact and Open Office hours

  We can be reached through irc, email and bugzilla
  https://sourceware.org/mission.html#organization

  There is also a fediverse account for for announcements, notices
  about downtime and temporary issues with our network.
  https://fosstodon.org/@sourceware

  Every second Friday of the month is the Sourceware Overseers Open
  Office hour in #overseers on irc.libera.chat from 16:00 till 17:00
  UTC. Please feel free to drop by with any Sourceware services and
  hosting questions.

  If you aren't already and want to keep up to date on Sourceware
  infrastructure services then please also subscribe to the overseers
  mailinglist https://sourceware.org/mailman/listinfo/overseers

  Do you or your company want to sponsor Sourceware plans financially
  https://sourceware.org/sourceware-security-vision.html#plans
  donate hardware or services then contact us at sp...@so...

Sourceware PLC,

 Frank Ch. Eigler, Ian Kelling, Ian Lance Taylor, Tom Tromey,
 Jon Turney, Mark J. Wielaard and Elena Zannoni

[Valgrind-developers] [valgrind] README_DEVELOPERS: change a few path names for consistency

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=b7ce61bec4c7c86cec0b2896d62c8b3a73015610

commit b7ce61bec4c7c86cec0b2896d62c8b3a73015610
Author: Florian Krohm <fl...@ei...>
Date:   Mon Jul 28 09:51:55 2025 +0000

    README_DEVELOPERS: change a few path names for consistency

Diff:
---
 README_DEVELOPERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README_DEVELOPERS b/README_DEVELOPERS
index a3360580eb..fc5a801295 100644
--- a/README_DEVELOPERS
+++ b/README_DEVELOPERS
@@ -271,12 +271,12 @@ without too much problem by following these steps:
       export VALGRIND_LIB=$DIR/.in_place
 
     VALGRIND_LIB is where the default.supp and vgpreload_ libraries
-    are found (which is under /usr/libexec/valgrind for an installed
+    are found (which is under /usr/local/libexec/valgrind for an installed
     version).
 
 (2) Run gdb on the tool executable.  Eg:
 
-      gdb /usr/local/lib/valgrind/lackey-ppc32-linux
+      gdb /usr/local/libexec/valgrind/lackey-ppc32-linux
 
     or

Re: [Valgrind-developers] none tool under gdb

[Florian K. <fl...@ei...>]

Hi Paul,

thanks for helping me out.

On 27.07.25 18:23, Paul Floyd via Valgrind-developers wrote:
> 
> I think that you are just missing the --tool=none argument to the tool.

Indeed, it just didn't occur to me that I'd have to specify --tool=none when I 
had explicitly launched none-amd64-linux...

Might be worth a warning when VG_(clo_toolname) and the launched tool executable 
are not in synch..

   Florian

Re: [Valgrind-developers] README_DEVELOPERS inconsistent paths

[Paul F. <pj...@wa...>]

On 7/27/25 14:58, Florian Krohm wrote:
> The way VALGRIND_LAUNCHER is defined in README_DEVELOPERS line 274 
> suggests that --prefix=/usr/local was used when configuring.
> But then VALGRIND_LIB needs adjustment and the path to the tool exec 
> as well.
> At least that is what I observe. 


Yes I think that both those changes should be made. configure defaults 
to /usr/local, and libexec is right (not lib).

The paths may be different for system installed packages. FreeBSD uses 
/usr/local but Solaris just uses /usr as does Debian.

A+

Paul

Re: [Valgrind-developers] none tool under gdb

[Paul F. <pj...@wa...>]

On 7/27/25 15:24, Florian Krohm wrote:
>
> I suppose people rarely debug the none tool. I found out because the 
> iropt tester runs under "none" auspices...
> Off-hand I have no idea what might be causing it.
>
Hi Florian

I think that you are just missing the --tool=none argument to the tool.

Once loaded in gdb you need to use something like

run --tool=none ./foo

(or start gdb with --args or use set args before the run).

If you leave out the "--tool=none" it will default to the memcheck 
options but with none as the tool. This isn't just a gdb thing. For 
instance (with the VALGRIND_* env vars set)

./.in_place/none-amd64-freebsd -q ./foo
--2850-- VG_USERREQ__CLIENT_CALL1: func=0x0
0x0

(I don't know why I'm getting a USERREQ moan, it's caused by an ifunc)

and

./.in_place/none-amd64-freebsd -q --tool=none ./foo
0x4e09000


A+

Paul

[Valgrind-developers] none tool under gdb

[Florian K. <fl...@ei...>]

Consider this program:

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
    return fprintf(stderr, "%p\n", (void *)malloc(10));
}

Compile this into foo and run in the root of the source tree:

./coregrind/valgrind --tool=none ./foo

prints some address as expected.

Running this under gdb like so:

export VALGRIND_LAUNCHER=./coregrind/valgrind
export VALGRIND_LIB=./.in_place
gdb -q -n ./.in_place/none-amd64-linux

memory allocation fails and the output is   (nil)

This is unexpected.. and not new...
Release 3.18.1 behaves the same way.. Which makes me think, that maybe I'm 
missing something?

I suppose people rarely debug the none tool. I found out because the iropt 
tester runs under "none" auspices...
Off-hand I have no idea what might be causing it.

    Florian

[Valgrind-developers] README_DEVELOPERS inconsistent paths

[Florian K. <fl...@ei...>]

The way VALGRIND_LAUNCHER is defined in README_DEVELOPERS line 274 suggests that 
--prefix=/usr/local was used when configuring.
But then VALGRIND_LIB needs adjustment and the path to the tool exec as well.
At least that is what I observe.


diff --git a/README_DEVELOPERS b/README_DEVELOPERS
index a3360580e..fc5a80129 100644
--- a/README_DEVELOPERS
+++ b/README_DEVELOPERS
@@ -271,12 +271,12 @@ without too much problem by following these steps:
        export VALGRIND_LIB=$DIR/.in_place

      VALGRIND_LIB is where the default.supp and vgpreload_ libraries
-    are found (which is under /usr/libexec/valgrind for an installed
+    are found (which is under /usr/local/libexec/valgrind for an installed
      version).

  (2) Run gdb on the tool executable.  Eg:

-      gdb /usr/local/lib/valgrind/lackey-ppc32-linux
+      gdb /usr/local/libexec/valgrind/lackey-ppc32-linux

      or

[Valgrind-developers] [valgrind] iropt-test: .vgtest tweaks

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=6d4745a6c9c0596f664a4b1b4c7c5ac815fde7d4

commit 6d4745a6c9c0596f664a4b1b4c7c5ac815fde7d4
Author: Florian Krohm <fl...@ei...>
Date:   Fri Jul 25 21:20:47 2025 +0000

    iropt-test: .vgtest tweaks

Diff:
---
 none/tests/iropt-test/iropt-test-sec.vgtest | 3 ++-
 none/tests/iropt-test/iropt-test.vgtest     | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/none/tests/iropt-test/iropt-test-sec.vgtest b/none/tests/iropt-test/iropt-test-sec.vgtest
index e8d4cc7025..38ada46550 100644
--- a/none/tests/iropt-test/iropt-test-sec.vgtest
+++ b/none/tests/iropt-test/iropt-test-sec.vgtest
@@ -1,4 +1,5 @@
 prog: iropt-test
 prereq: test -x iropt-test-sec
+#args: -v -v -r10
+args: -r100
 vgopts: -q --vex-guest-chase=no
-
diff --git a/none/tests/iropt-test/iropt-test.vgtest b/none/tests/iropt-test/iropt-test.vgtest
index de31de6046..9136b105c9 100644
--- a/none/tests/iropt-test/iropt-test.vgtest
+++ b/none/tests/iropt-test/iropt-test.vgtest
@@ -2,4 +2,3 @@ prog: iropt-test
 #args: -v -v -r10
 args: -r100
 vgopts: -q --vex-guest-chase=no
-

[Valgrind-developers] [valgrind] iropt-test: Refactor code

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=5a159cf245ff4fec2fd148c899ce7a0380ca3d04

commit 5a159cf245ff4fec2fd148c899ce7a0380ca3d04
Author: Florian Krohm <fl...@ei...>
Date:   Fri Jul 25 21:11:57 2025 +0000

    iropt-test: Refactor code
    
    Refactor code in preparation for running each testcase twice: once
    with constant folding and once without.
    - remove function print_opnd
    - remove function complain
    - factor out function get_expected_value
    - checking the result moved to valgrind_execute_test
    - make IRICB a static global in valgrind.c
    - new_iricb now returns a pointer to it

Diff:
---
 none/tests/iropt-test/binary.c   | 41 ++++++++---------------------------
 none/tests/iropt-test/main.c     |  4 ++--
 none/tests/iropt-test/unary.c    | 38 +++++++-------------------------
 none/tests/iropt-test/util.c     | 29 -------------------------
 none/tests/iropt-test/valgrind.c | 47 +++++++++++++++++++++++++++++++---------
 none/tests/iropt-test/vtest.h    |  6 ++---
 6 files changed, 58 insertions(+), 107 deletions(-)

diff --git a/none/tests/iropt-test/binary.c b/none/tests/iropt-test/binary.c
index 62c9faeb35..0d40a4da3e 100644
--- a/none/tests/iropt-test/binary.c
+++ b/none/tests/iropt-test/binary.c
@@ -26,7 +26,7 @@
 #include <stdint.h>     // UINT64_MAX
 #include "vtest.h"
 
-static void check_result(const irop_t *, const test_data_t *);
+static uint64_t get_expected_value(const irop_t *, const test_data_t *);
 static void run_tests(const irop_t *, test_data_t *);
 static void run_shift_tests(const irop_t *, test_data_t *);
 static int  is_shift_op(IROp);
@@ -59,8 +59,7 @@ run_selected_tests(const irop_t *op, test_data_t *data)
 
          if (is_division_op(op->op) && opnd_r->value == 0) continue;
 
-         valgrind_execute_test(op, data);
-         check_result(op, data);
+         valgrind_execute_test(op, data, get_expected_value(op, data));
       }
    }
 }
@@ -79,8 +78,7 @@ run_random_tests(const irop_t *op, test_data_t *data)
 
       if (is_division_op(op->op) && opnd_r->value == 0) continue;
 
-      valgrind_execute_test(op, data);
-      check_result(op, data);
+      valgrind_execute_test(op, data, get_expected_value(op, data));
    }
 }
 
@@ -101,8 +99,7 @@ run_shift_tests(const irop_t *op, test_data_t *data)
       for (unsigned j = 0; j < max_shift_amount; ++j) {
          opnd_r->value = j;
 
-         valgrind_execute_test(op, data);
-         check_result(op, data);
+         valgrind_execute_test(op, data, get_expected_value(op, data));
       }
    }
 
@@ -111,8 +108,7 @@ run_shift_tests(const irop_t *op, test_data_t *data)
       opnd_l->value = get_random_value(opnd_l->type);
       opnd_r->value = get_random_value(opnd_r->type) & max_shift_amount;
 
-      valgrind_execute_test(op, data);
-      check_result(op, data);
+      valgrind_execute_test(op, data, get_expected_value(op, data));
    }
 }
 
@@ -125,11 +121,10 @@ run_tests(const irop_t *op, test_data_t *data)
 }
 
 
-/* Check the result of a binary operation. */
-static void
-check_result(const irop_t *op, const test_data_t *data)
+/* Compute the expected result of a binary operation. */
+static uint64_t
+get_expected_value(const irop_t *op, const test_data_t *data)
 {
-   uint64_t result = data->result.value;
    uint64_t opnd_l = data->opnds[0].value;
    uint64_t opnd_r = data->opnds[1].value;
    uint64_t expected;
@@ -384,25 +379,7 @@ check_result(const irop_t *op, const test_data_t *data)
       panic(__func__);
    }
 
-   if (verbose > 1) {
-      printf("expected:  value = ");
-      print_value(stdout, expected, bitsof_irtype(data->result.type));
-      printf("\n");
-   }
-
-   int ok = 1;
-   switch (data->result.type) {
-   case Ity_I1:  ok = result == expected; break;
-   case Ity_I8:  ok = result == expected; break;
-   case Ity_I16: ok = result == expected; break;
-   case Ity_I32: ok = result == expected; break;
-   case Ity_I64: ok = result == expected; break;
-   default:
-      panic(__func__);
-   }
-
-   if (! ok)
-      complain(op, data, expected);
+   return expected;
 }
 
 
diff --git a/none/tests/iropt-test/main.c b/none/tests/iropt-test/main.c
index 9f1cc5083a..8fcf9fb453 100644
--- a/none/tests/iropt-test/main.c
+++ b/none/tests/iropt-test/main.c
@@ -81,9 +81,9 @@ main(int argc, char *argv[])
 
       test_data_t *data = new_test_data(op);
 
-      IRICB iricb = new_iricb(op, data);
+      IRICB *iricb = new_iricb(op, data);
 
-      valgrind_vex_init_for_iri(&iricb);
+      valgrind_vex_init_for_iri(iricb);
 
       switch (op->num_opnds) {
       case 1:
diff --git a/none/tests/iropt-test/unary.c b/none/tests/iropt-test/unary.c
index e6554cef60..fa961f28a7 100644
--- a/none/tests/iropt-test/unary.c
+++ b/none/tests/iropt-test/unary.c
@@ -27,9 +27,9 @@
 #include <stdint.h>     // UINT64_MAX
 #include "vtest.h"
 
-static void check_result(const irop_t *, const test_data_t *);
 static void run_selected_tests(const irop_t *, test_data_t *);
 static void run_random_tests(const irop_t *, test_data_t *);
+static uint64_t get_expected_value(const irop_t *, const test_data_t *);
 static uint64_t left(uint64_t, unsigned);
 static uint32_t popcount(uint64_t);
 static uint32_t clz(uint64_t, unsigned);
@@ -49,8 +49,7 @@ test_unary_op(const irop_t *op, test_data_t *data)
       for (unsigned i = 0; i <= max; ++i) {
          opnd->value = i;
 
-         valgrind_execute_test(op, data);
-         check_result(op, data);
+         valgrind_execute_test(op, data, get_expected_value(op, data));
       }
       break;
    }
@@ -78,8 +77,7 @@ run_selected_tests(const irop_t *op, test_data_t *data)
    for (unsigned i = 0; i < num_val; ++i) {
       opnd->value = values[i];
 
-      valgrind_execute_test(op, data);
-      check_result(op, data);
+      valgrind_execute_test(op, data, get_expected_value(op, data));
    }
 }
 
@@ -93,17 +91,15 @@ run_random_tests(const irop_t *op, test_data_t *data)
    for (unsigned i = 0; i < num_random_tests; ++i) {
       opnd->value = get_random_value(opnd->type);
 
-      valgrind_execute_test(op, data);
-      check_result(op, data);
+      valgrind_execute_test(op, data, get_expected_value(op, data));
    }
 }
 
 
-/* Check the result of a unary operation. */
-static void
-check_result(const irop_t *op, const test_data_t *data)
+/* Compute the expected result of a unary operation. */
+static uint64_t
+get_expected_value(const irop_t *op, const test_data_t *data)
 {
-   uint64_t result = data->result.value;
    uint64_t opnd   = data->opnds[0].value;
    uint64_t expected;
 
@@ -209,25 +205,7 @@ check_result(const irop_t *op, const test_data_t *data)
       panic("%s: operator %s not handled\n", __func__, op->name);
    }
 
-   if (verbose > 1) {
-      printf("expected:  value = ");
-      print_value(stdout, expected, bitsof_irtype(data->result.type));
-      printf("\n");
-   }
-
-   int ok = 1;
-   switch (data->result.type) {
-   case Ity_I1:  ok = result == expected; break;
-   case Ity_I8:  ok = result == expected; break;
-   case Ity_I16: ok = result == expected; break;
-   case Ity_I32: ok = result == expected; break;
-   case Ity_I64: ok = result == expected; break;
-   default:
-      panic(__func__);
-   }
-
-   if (! ok)
-      complain(op, data, expected);
+   return expected;
 }
 
 
diff --git a/none/tests/iropt-test/util.c b/none/tests/iropt-test/util.c
index 18b671114c..f34936611e 100644
--- a/none/tests/iropt-test/util.c
+++ b/none/tests/iropt-test/util.c
@@ -44,27 +44,6 @@ panic(const char *fmt, ...)
 }
 
 
-/* Issue a complaint because the result of an operation differs from what
-   was expected. */
-void
-complain(const irop_t *op, const test_data_t *data, uint64_t expected)
-{
-   fprintf(stderr, "*** Incorrect result for operator %s\n", op->name);
-
-   for (unsigned i = 0; i < op->num_opnds; ++i) {
-      fprintf(stderr, "    opnd %u:  ", i);
-      print_opnd(stderr, &data->opnds[i]);
-      fprintf(stderr, "\n");
-   }
-   fprintf(stderr, "    result:  ");
-   print_opnd(stderr, &data->result);
-   fprintf(stderr, "\n");
-   fprintf(stderr, "    expect:  ");
-   print_value(stderr, expected, bitsof_irtype(op->result_type));
-   fprintf(stderr, "\n");
-}
-
-
 void
 print_value(FILE *fp, uint64_t val, unsigned num_bits)
 {
@@ -83,14 +62,6 @@ print_value(FILE *fp, uint64_t val, unsigned num_bits)
 }
 
 
-void
-print_opnd(FILE *fp, const opnd_t *opnd)
-{
-   fprintf(fp, "value = ");
-   print_value(fp, opnd->value, bitsof_irtype(opnd->type));
-}
-
-
 unsigned
 bitsof_irtype(IRType ty)
 {
diff --git a/none/tests/iropt-test/valgrind.c b/none/tests/iropt-test/valgrind.c
index e482909b16..004c8a09a4 100644
--- a/none/tests/iropt-test/valgrind.c
+++ b/none/tests/iropt-test/valgrind.c
@@ -26,9 +26,10 @@
 #include "valgrind.h"  // VALGRIND_VEX_INJECT_IR
 #include "vtest.h"
 
+static IRICB iricb;
 
 /* Return a completely initialised control block */
-IRICB
+IRICB *
 new_iricb(const irop_t *op, test_data_t *data)
 {
    IRICB_iropt_payload cb;
@@ -45,7 +46,10 @@ new_iricb(const irop_t *op, test_data_t *data)
 
    cb.num_operands = op->num_opnds;
 
-   return (IRICB) { .kind = IRICB_iropt, .iropt = cb };
+   iricb.kind  = IRICB_iropt;
+   iricb.iropt = cb;
+
+   return &iricb;
 }
 
 
@@ -69,24 +73,47 @@ valgrind_vex_inject_ir(void)
 /* Execute the test under valgrind. Well, yes, we're not really executing
    it here, just preparing for it... */
 void
-valgrind_execute_test(const irop_t *op, test_data_t *data)
+valgrind_execute_test(const irop_t *op, test_data_t *data, uint64_t expected)
 {
-   if (verbose > 1)
+   if (verbose > 1) {
       printf("---------- Running a test\n");
 
-   for (unsigned i = 0; i < op->num_opnds; ++i) {
-      if (verbose > 1) {
-         printf("opnd #%u:   ", i);
-         print_opnd(stdout, &data->opnds[i]);
+      for (unsigned i = 0; i < op->num_opnds; ++i) {
+         const opnd_t *opnd = data->opnds + i;
+         printf("opnd %u:    value = ", i);
+         print_value(stdout, opnd->value, bitsof_irtype(opnd->type));
          printf("\n");
       }
    }
 
    valgrind_vex_inject_ir();
+   uint64_t result = data->result.value;
 
+   unsigned num_result_bits = bitsof_irtype(data->result.type);
    if (verbose > 1) {
-      printf("result:    ");
-      print_opnd(stdout, &data->result);
+      printf("result:    value = ");
+      print_value(stdout, result, num_result_bits);
       printf("\n");
+      printf("expected:  value = ");
+      print_value(stdout, expected, num_result_bits);
+      printf("\n");
+   }
+
+   /* Check result */
+   if (result != expected) {
+      fprintf(stderr, "*** Incorrect result for operator %s\n", op->name);
+
+      for (unsigned i = 0; i < op->num_opnds; ++i) {
+         const opnd_t *opnd = data->opnds + i;
+         fprintf(stderr, "    opnd %u:  ", i);
+         print_value(stderr, opnd->value, bitsof_irtype(opnd->type));
+         fprintf(stderr, "\n");
+      }
+      fprintf(stderr, "    result:  ");
+      print_value(stderr, result, num_result_bits);
+      fprintf(stderr, "\n");
+      fprintf(stderr, "    expect:  ");
+      print_value(stderr, expected, num_result_bits);
+      fprintf(stderr, "\n");
    }
 }
diff --git a/none/tests/iropt-test/vtest.h b/none/tests/iropt-test/vtest.h
index 47b397f681..f31f3c3ce4 100644
--- a/none/tests/iropt-test/vtest.h
+++ b/none/tests/iropt-test/vtest.h
@@ -69,19 +69,17 @@ typedef struct {
 
 
 /* Function prototypes */
-void print_opnd(FILE *, const opnd_t *);
 void print_value(FILE *, uint64_t, unsigned);
 
 void test_unary_op(const irop_t *, test_data_t *);
 void test_binary_op(const irop_t *, test_data_t *);
 
 void valgrind_vex_init_for_iri(IRICB *);
-void valgrind_execute_test(const irop_t *, test_data_t *);
+void valgrind_execute_test(const irop_t *, test_data_t *, uint64_t);
 
-IRICB new_iricb(const irop_t *, test_data_t *);
+IRICB *new_iricb(const irop_t *, test_data_t *);
 
 void panic(const char *, ...) __attribute__((noreturn));
-void complain(const irop_t *, const test_data_t *, uint64_t expected);
 
 unsigned bitsof_irtype(IRType);
 uint64_t get_random_value(IRType);

[Valgrind-developers] [valgrind] FreeBSD regtest: update 32bit scalar expected

[Paul F. <pa...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=1b5773d82046701f66697f68599acf0753db0402

commit 1b5773d82046701f66697f68599acf0753db0402
Author: Paul Floyd <pj...@wa...>
Date:   Fri Jul 25 07:48:25 2025 +0200

    FreeBSD regtest: update 32bit scalar expected

Diff:
---
 memcheck/tests/freebsd/scalar.stderr.exp-x86 | 30 ++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/memcheck/tests/freebsd/scalar.stderr.exp-x86 b/memcheck/tests/freebsd/scalar.stderr.exp-x86
index ea5abb9c61..51ae7e94c0 100644
--- a/memcheck/tests/freebsd/scalar.stderr.exp-x86
+++ b/memcheck/tests/freebsd/scalar.stderr.exp-x86
@@ -2515,6 +2515,19 @@ Syscall param sigwaitinfo(info) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
+---------------------------------------------------------
+346:         SYS_sigwaitinfo (NULL info) 2s 1m
+---------------------------------------------------------
+Syscall param sigwaitinfo(set) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwaitinfo(info) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwaitinfo(set) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 ---------------------------------------------------------
 347:      SYS___acl_get_file 3s 2m
 ---------------------------------------------------------
@@ -3369,6 +3382,23 @@ Syscall param sigwait(sig) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
+---------------------------------------------------------
+429:             SYS_sigwait (NULL ags) 2s 2m
+---------------------------------------------------------
+Syscall param sigwait(set) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwait(sig) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwait(set) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
+Syscall param sigwait(sig) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 ---------------------------------------------------------
 431:            SYS_thr_exit other
 ---------------------------------------------------------

[Valgrind-developers] [valgrind] FreeBSD syscall: improve sigwait and sigwaitinfo wrapper.

[Paul F. <pa...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=359e33acaf8cded1312e75709b91d4f8156ae574

commit 359e33acaf8cded1312e75709b91d4f8156ae574
Author: Paul Floyd <pj...@wa...>
Date:   Thu Jul 24 22:45:01 2025 +0200

    FreeBSD syscall: improve sigwait and sigwaitinfo wrapper.
    
    Both take two pointers. We were allowing null pointers for all
    of them. Only the 2nd argument of sigwaitinfo, info, is
    allowed to be NULL. Update the scalar test with some NULL
    arguments for these syscalls.

Diff:
---
 coregrind/m_syswrap/syswrap-freebsd.c    | 23 ++++++++++-------------
 memcheck/tests/freebsd/scalar.c          | 10 +++++++++-
 memcheck/tests/freebsd/scalar.stderr.exp | 30 ++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c
index ac371e125a..08c4ec3c3c 100644
--- a/coregrind/m_syswrap/syswrap-freebsd.c
+++ b/coregrind/m_syswrap/syswrap-freebsd.c
@@ -3317,9 +3317,7 @@ PRE(sys_sigwaitinfo)
          ARG1,ARG2);
    PRE_REG_READ2(int, "sigwaitinfo",
                  const vki_sigset_t *, set, vki_siginfo_t *, info);
-   if (ARG1 != 0) {
-      PRE_MEM_READ(  "sigwaitinfo(set)",  ARG1, sizeof(vki_sigset_t));
-   }
+   PRE_MEM_READ(  "sigwaitinfo(set)",  ARG1, sizeof(vki_sigset_t));
    if (ARG2 != 0) {
       PRE_MEM_WRITE( "sigwaitinfo(info)", ARG2, sizeof(vki_siginfo_t) );
    }
@@ -4099,21 +4097,20 @@ PRE(sys_sigwait)
          ARG1,ARG2);
    PRE_REG_READ2(int, "sigwait",
                  const vki_sigset_t *, set, int *, sig);
-   if (ARG1 != 0) {
-      PRE_MEM_READ(  "sigwait(set)",  ARG1, sizeof(vki_sigset_t));
-      vki_sigset_t* set = (vki_sigset_t*)ARG1;
-      if (ML_(safe_to_deref)(set, sizeof(vki_sigset_t))) {
-         *flags |= SfMayBlock;
-      }
-   }
-   if (ARG2 != 0) {
-      PRE_MEM_WRITE( "sigwait(sig)", ARG2, sizeof(int));
+   PRE_MEM_READ(  "sigwait(set)",  ARG1, sizeof(vki_sigset_t));
+   vki_sigset_t* set = (vki_sigset_t*)ARG1;
+   if (ML_(safe_to_deref)(set, sizeof(vki_sigset_t))) {
+      *flags |= SfMayBlock;
    }
+   PRE_MEM_WRITE( "sigwait(sig)", ARG2, sizeof(int));
 }
 
+// sigwait doesn't follow the norm of returning -1 on error
+// instead it returns errno if there is an error
 POST(sys_sigwait)
 {
-   if (RES == 0 && ARG2 != 0) {
+   if (RES == 0)
+   {
       POST_MEM_WRITE( ARG2, sizeof(int));
    }
 }
diff --git a/memcheck/tests/freebsd/scalar.c b/memcheck/tests/freebsd/scalar.c
index 9c67888f98..234e649f78 100644
--- a/memcheck/tests/freebsd/scalar.c
+++ b/memcheck/tests/freebsd/scalar.c
@@ -1271,7 +1271,11 @@ int main(void)
 
    /* SYS_sigwaitinfo             346 */
    GO(SYS_sigwaitinfo, "2s 2m");
-   SY(SYS_sigwaitinfo, x0+1, x0+2, x0+3); FAIL;
+   SY(SYS_sigwaitinfo, x0+1, x0+2); FAIL;
+
+   GO(SYS_sigwaitinfo, "(NULL info) 2s 1m");
+   SY(SYS_sigwaitinfo, x0, x0); FAIL;
+
 
    /* SYS___acl_get_file          347 */
    GO(SYS___acl_get_file, "3s 2m");
@@ -1549,6 +1553,10 @@ int main(void)
    SY(SYS_sigwait, x0+1, x0+2); SUCC;
    assert(res == EFAULT);
 
+   GO(SYS_sigwait, "(NULL ags) 2s 2m");
+   SY(SYS_sigwait, x0, x0); SUCC;
+   assert(res == EFAULT);
+
    // thr_create                  430
 
    /* SYS_thr_exit                431 */
diff --git a/memcheck/tests/freebsd/scalar.stderr.exp b/memcheck/tests/freebsd/scalar.stderr.exp
index dbe79c6e84..ae9c15f6a8 100644
--- a/memcheck/tests/freebsd/scalar.stderr.exp
+++ b/memcheck/tests/freebsd/scalar.stderr.exp
@@ -2509,6 +2509,19 @@ Syscall param sigwaitinfo(info) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
+---------------------------------------------------------
+346:         SYS_sigwaitinfo (NULL info) 2s 1m
+---------------------------------------------------------
+Syscall param sigwaitinfo(set) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwaitinfo(info) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwaitinfo(set) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 ---------------------------------------------------------
 347:      SYS___acl_get_file 3s 2m
 ---------------------------------------------------------
@@ -3360,6 +3373,23 @@ Syscall param sigwait(sig) points to unaddressable byte(s)
    ...
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
+---------------------------------------------------------
+429:             SYS_sigwait (NULL ags) 2s 2m
+---------------------------------------------------------
+Syscall param sigwait(set) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwait(sig) contains uninitialised byte(s)
+   ...
+
+Syscall param sigwait(set) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
+Syscall param sigwait(sig) points to unaddressable byte(s)
+   ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+
 ---------------------------------------------------------
 431:            SYS_thr_exit other
 ---------------------------------------------------------

[Valgrind-developers] [valgrind] README_DEVELOPERS: more on scalar test

[Paul F. <pa...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=0369a3b9c7dd216b660615d91ae3836f69d674fa

commit 0369a3b9c7dd216b660615d91ae3836f69d674fa
Author: Paul Floyd <pj...@wa...>
Date:   Thu Jul 24 22:44:09 2025 +0200

    README_DEVELOPERS: more on scalar test
    
    One typo and add an example.

Diff:
---
 README_DEVELOPERS | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/README_DEVELOPERS b/README_DEVELOPERS
index 5bcb8ea87e..a3360580eb 100644
--- a/README_DEVELOPERS
+++ b/README_DEVELOPERS
@@ -197,13 +197,27 @@ The last file to change is .gitignore in the top directory. Add a new entry,
 for example "/tool/tests/newtest".
 
 The 'scalar' tests are something of a special case. Scalar in this sense
-refers to the registers (or stack slots) used to pas in arguments. These tests
+refers to the registers (or stack slots) used to pass in arguments. These tests
 directly use the 'syscall' syscall via a macro, SY. They make little effort
 to use the sysall in a realistic manner. Rather, the objective is to
 exhaustively test all of the arguemnts and referenced memory of syscalls.
+The variable 'x0' is a long integer, containing the value of 0 but
+also uninitialised. It can be used on its own or with some other value
+to ensure that all of the syscall arguemts are uninitialised.
 A second macro, GO, is used precede the syscall (and subsequent errors)
 with a header. The GO string includes the name of the syscall, a count of
-expected scalar errors and a count of memory errors.
+expected scalar errors and a count of memory errors. The tests are usually
+followed by the FAIL macro, which ensures that the syscall failed.
+An example scalar test is
+
+   /* SYS_link                    9 */
+   GO(SYS_link, "2s 2m");
+   SY(SYS_link, x0, x0); FAIL;
+
+
+This syscall takes two strings so the expected errors are
+2 scalar (for the pointer arguments) and 2 memory (for
+the strings themselves).
 
 When your test is done check for mistakes in Makefile.am.
 In the top directory run

[Valgrind-developers] [valgrind] README_DEVELOPERS: add some description of scalar tests

[Paul F. <pa...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=13eb7509bf22b67ccca1b521e10f2984985d1b5d

commit 13eb7509bf22b67ccca1b521e10f2984985d1b5d
Author: Paul Floyd <pj...@wa...>
Date:   Thu Jul 24 22:41:11 2025 +0200

    README_DEVELOPERS: add some description of scalar tests

Diff:
---
 README_DEVELOPERS | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/README_DEVELOPERS b/README_DEVELOPERS
index 4444a17500..5bcb8ea87e 100644
--- a/README_DEVELOPERS
+++ b/README_DEVELOPERS
@@ -196,7 +196,18 @@ Make sure that the test runs and passes.
 The last file to change is .gitignore in the top directory. Add a new entry,
 for example "/tool/tests/newtest".
 
-Check for mistakes in Makefile.am. In the top directory run
+The 'scalar' tests are something of a special case. Scalar in this sense
+refers to the registers (or stack slots) used to pas in arguments. These tests
+directly use the 'syscall' syscall via a macro, SY. They make little effort
+to use the sysall in a realistic manner. Rather, the objective is to
+exhaustively test all of the arguemnts and referenced memory of syscalls.
+A second macro, GO, is used precede the syscall (and subsequent errors)
+with a header. The GO string includes the name of the syscall, a count of
+expected scalar errors and a count of memory errors.
+
+When your test is done check for mistakes in Makefile.am.
+In the top directory run
+
     make post-regtest-checks
 
 You should only see

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Florian K. <fl...@ei...>]

Hi Mark,

On 23.07.25 23:46, Mark Wielaard wrote:
> 
> If I am interpreting
> https://en.wikipedia.org/wiki/X86_Bit_manipulation_instruction_set#ABM_(Advanced_Bit_Manipulation)
> correctly then I think this is correct for x86-lzcnt and amd64-lzcnt,
> because the ABM cpuid bit is the same as what we call the LZCNT bit. I
> don't know about amd64-fma4, I believe only AMD ever implemented that.
> 

I checked in the patch but left amd64-fma4 alone and did not change it.
Turns out amd64-fma4 is not used at all but there is a spurious reference
to it in none/tests/amd64/fma4.c

Cheers
   Florian

[Valgrind-developers] [valgrind] tests/x86_amd_features.c: followup to 4810c14b5d

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=714b71ea5dc552d297b8759e18a9006ff3802d18

commit 714b71ea5dc552d297b8759e18a9006ff3802d18
Author: Florian Krohm <fl...@ei...>
Date:   Thu Jul 24 09:43:04 2025 +0000

    tests/x86_amd_features.c: followup to 4810c14b5d
    
    Do not rely on hardware identifying itself in a particular way.
    Enable lzcnt for non-AMD hardware.

Diff:
---
 tests/x86_amd64_features.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
index 5a95e06df3..955cca13b0 100644
--- a/tests/x86_amd64_features.c
+++ b/tests/x86_amd64_features.c
@@ -102,7 +102,6 @@ static Bool go(char* cpu)
    } else if ( strcmp( cpu, "x86-lzcnt" ) == 0 ) {
      level = 0x80000001;
      cmask = 1 << 5;
-     require_amd = True;
 #if defined(VGA_amd64)
    } else if ( strcmp( cpu, "amd64-sse3" ) == 0 ) {
      level = 1;
@@ -119,7 +118,6 @@ static Bool go(char* cpu)
    } else if ( strcmp( cpu, "amd64-lzcnt" ) == 0 ) {
      level = 0x80000001;
      cmask = 1 << 5;
-     require_amd = True;
    } else if ( strcmp( cpu, "amd64-sse42" ) == 0 ) {
      level = 1;
      cmask = 1 << 20;
@@ -148,8 +146,7 @@ static Bool go(char* cpu)
    assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
    assert( !(cmask == 0 && dmask == 0 && bmask == 0) );
 
-   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
-       !vendorStringEquals("GenuineIntel"))
+   if (require_amd && !vendorStringEquals("AuthenticAMD"))
       return FEATURE_NOT_PRESENT;
       // regardless of what that feature actually is

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Mark W. <ma...@kl...>]

Hi Florian,

On Wed, Jul 23, 2025 at 11:22:33PM +0200, Florian Krohm wrote:
> >So it might be that the x86-lzcnt (and amd64-lzcnt?) checks need to be
> >rewritten to detect the cpu supports them (and executes them as LZCNT,
> >not as BSR). But I am don't think just adding
> >!vendorStringEquals("GenuineIntel") is the correct way to do that.
> >
> 
> As discussed on IRC here's a revised version. Regtested. OK?

If I am interpreting
https://en.wikipedia.org/wiki/X86_Bit_manipulation_instruction_set#ABM_(Advanced_Bit_Manipulation)
correctly then I think this is correct for x86-lzcnt and amd64-lzcnt,
because the ABM cpuid bit is the same as what we call the LZCNT bit. I
don't know about amd64-fma4, I believe only AMD ever implemented that.

Cheers,

Mark

> diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
> index 5a95e06df..7b58c1e92 100644
> --- a/tests/x86_amd64_features.c
> +++ b/tests/x86_amd64_features.c
> @@ -102,7 +102,6 @@ static Bool go(char* cpu)
>     } else if ( strcmp( cpu, "x86-lzcnt" ) == 0 ) {
>       level = 0x80000001;
>       cmask = 1 << 5;
> -     require_amd = True;
>  #if defined(VGA_amd64)
>     } else if ( strcmp( cpu, "amd64-sse3" ) == 0 ) {
>       level = 1;
> @@ -119,7 +118,6 @@ static Bool go(char* cpu)
>     } else if ( strcmp( cpu, "amd64-lzcnt" ) == 0 ) {
>       level = 0x80000001;
>       cmask = 1 << 5;
> -     require_amd = True;
>     } else if ( strcmp( cpu, "amd64-sse42" ) == 0 ) {
>       level = 1;
>       cmask = 1 << 20;
> @@ -130,7 +128,6 @@ static Bool go(char* cpu)
>     } else if (strcmp (cpu,  "amd64-fma4" ) == 0) {
>       level = 0x80000001;
>       cmask = 1 << 16;
> -     require_amd = True;
>     } else if (strcmp (cpu,  "amd64-f16c" ) == 0) {
>        level = 1;
>        cmask = 1 << 29;
> @@ -148,8 +145,7 @@ static Bool go(char* cpu)
>     assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
>     assert( !(cmask == 0 && dmask == 0 && bmask == 0) );
> 
> -   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
> -       !vendorStringEquals("GenuineIntel"))
> +   if (require_amd && !vendorStringEquals("AuthenticAMD"))
>        return FEATURE_NOT_PRESENT;
>        // regardless of what that feature actually is
> 

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Florian K. <fl...@ei...>]

On 23.07.25 22:29, Mark Wielaard wrote:
> Hi Florian,
> 
> On Wed, Jul 23, 2025 at 11:53:54AM +0200, Florian Krohm wrote:
>> While regtesting a patch for x86 to remove Iop_Clz32 (BZ 507033) I
>> noticed that the testcase none/tests/x86/lzcnt32 was not executed
>> due to lack of required prereqs. This of course is *the* interesting
>> testcase here.
>>
>> The reason it's not executed is because tests/x86_amd64_features.c has this:
>>
>>     if (require_amd && !vendorStringEquals("AuthenticAMD"))
>>        return FEATURE_NOT_PRESENT;
>>
>> My laptop identifies itself as "GenuineIntel". Therefore I propose
>> the regtested patch below. Anybody sees a problem with that?
> 
> I think the issue is that LZCNT really is (or was?) AMD only. If
> executed on Intel it behaves like BSR (which is encoded the same). So
> the check as written seems as intended.
> 
> But I believe since that test was written Intel introduced BMI, which
> includes LZCNT.
> 
> So it might be that the x86-lzcnt (and amd64-lzcnt?) checks need to be
> rewritten to detect the cpu supports them (and executes them as LZCNT,
> not as BSR). But I am don't think just adding
> !vendorStringEquals("GenuineIntel") is the correct way to do that.
> 

As discussed on IRC here's a revised version. Regtested. OK?

diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
index 5a95e06df..7b58c1e92 100644
--- a/tests/x86_amd64_features.c
+++ b/tests/x86_amd64_features.c
@@ -102,7 +102,6 @@ static Bool go(char* cpu)
     } else if ( strcmp( cpu, "x86-lzcnt" ) == 0 ) {
       level = 0x80000001;
       cmask = 1 << 5;
-     require_amd = True;
  #if defined(VGA_amd64)
     } else if ( strcmp( cpu, "amd64-sse3" ) == 0 ) {
       level = 1;
@@ -119,7 +118,6 @@ static Bool go(char* cpu)
     } else if ( strcmp( cpu, "amd64-lzcnt" ) == 0 ) {
       level = 0x80000001;
       cmask = 1 << 5;
-     require_amd = True;
     } else if ( strcmp( cpu, "amd64-sse42" ) == 0 ) {
       level = 1;
       cmask = 1 << 20;
@@ -130,7 +128,6 @@ static Bool go(char* cpu)
     } else if (strcmp (cpu,  "amd64-fma4" ) == 0) {
       level = 0x80000001;
       cmask = 1 << 16;
-     require_amd = True;
     } else if (strcmp (cpu,  "amd64-f16c" ) == 0) {
        level = 1;
        cmask = 1 << 29;
@@ -148,8 +145,7 @@ static Bool go(char* cpu)
     assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
     assert( !(cmask == 0 && dmask == 0 && bmask == 0) );

-   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
-       !vendorStringEquals("GenuineIntel"))
+   if (require_amd && !vendorStringEquals("AuthenticAMD"))
        return FEATURE_NOT_PRESENT;
        // regardless of what that feature actually is

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Mark W. <ma...@kl...>]

Hi Florian,

On Wed, Jul 23, 2025 at 11:53:54AM +0200, Florian Krohm wrote:
> While regtesting a patch for x86 to remove Iop_Clz32 (BZ 507033) I
> noticed that the testcase none/tests/x86/lzcnt32 was not executed
> due to lack of required prereqs. This of course is *the* interesting
> testcase here.
> 
> The reason it's not executed is because tests/x86_amd64_features.c has this:
> 
>    if (require_amd && !vendorStringEquals("AuthenticAMD"))
>       return FEATURE_NOT_PRESENT;
> 
> My laptop identifies itself as "GenuineIntel". Therefore I propose
> the regtested patch below. Anybody sees a problem with that?

I think the issue is that LZCNT really is (or was?) AMD only. If
executed on Intel it behaves like BSR (which is encoded the same). So
the check as written seems as intended.

But I believe since that test was written Intel introduced BMI, which
includes LZCNT.

So it might be that the x86-lzcnt (and amd64-lzcnt?) checks need to be
rewritten to detect the cpu supports them (and executes them as LZCNT,
not as BSR). But I am don't think just adding
!vendorStringEquals("GenuineIntel") is the correct way to do that.

Cheers,

Mark

[Valgrind-developers] [valgrind] tests/x86_amd_features.c: also recognise GenuineIntel

[Florian K. <fk...@so...>]

https://sourceware.org/cgit/valgrind/commit/?id=4810c14b5d636e758660a7cae931fa82071b86eb

commit 4810c14b5d636e758660a7cae931fa82071b86eb
Author: Florian Krohm <fl...@ei...>
Date:   Wed Jul 23 20:20:54 2025 +0000

    tests/x86_amd_features.c: also recognise GenuineIntel
    
    Enables running certain testcases on hardware that identifies itself
    as "GenuineIntel".

Diff:
---
 tests/x86_amd64_features.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
index 488f155b64..5a95e06df3 100644
--- a/tests/x86_amd64_features.c
+++ b/tests/x86_amd64_features.c
@@ -148,7 +148,8 @@ static Bool go(char* cpu)
    assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
    assert( !(cmask == 0 && dmask == 0 && bmask == 0) );
 
-   if (require_amd && !vendorStringEquals("AuthenticAMD"))
+   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
+       !vendorStringEquals("GenuineIntel"))
       return FEATURE_NOT_PRESENT;
       // regardless of what that feature actually is

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Florian K. <fl...@ei...>]

On 23.07.25 22:01, John Reiser wrote:
>> Cyrix, VIA and Transmeta have all gone. There is one Chinese c86 manufacturer.
> 
> The current release of Valgrind does run on 10-year-old Linux,
> so the universe of chips that old should be recognized.
> 

Not to worry. The patch does not remove any functionality and only affects 
running testcases.

   Florian

Re: [Valgrind-developers] tests/x86_amd64_features.c

[John R. <jr...@bi...>]

> Cyrix, VIA and Transmeta have all gone. There is one Chinese c86 
> manufacturer.

The current release of Valgrind does run on 10-year-old Linux,
so the universe of chips that old should be recognized.

Re: [Valgrind-developers] tests/x86_amd64_features.c

[Paul F. <pj...@wa...>]

On 7/23/25 11:53, Florian Krohm wrote:
>
> diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
> index 488f155b6..5a95e06df 100644
> --- a/tests/x86_amd64_features.c
> +++ b/tests/x86_amd64_features.c
> @@ -148,7 +148,8 @@ static Bool go(char* cpu)
>     assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
>     assert( !(cmask == 0 && dmask == 0 && bmask == 0) );
>
> -   if (require_amd && !vendorStringEquals("AuthenticAMD"))
> +   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
> +       !vendorStringEquals("GenuineIntel"))
>        return FEATURE_NOT_PRESENT;
>        // regardless of what that feature actually is 


Looks fine, but I don't know what is likely to ever present as neither 
AuthenticAMD nor GenuineIntel.

Cyrix, VIA and Transmeta have all gone. There is one Chinese c86 
manufacturer.


A+

Paul

[Valgrind-developers] tests/x86_amd64_features.c

[Florian K. <fl...@ei...>]

While regtesting a patch for x86 to remove Iop_Clz32 (BZ 507033) I noticed that 
the testcase none/tests/x86/lzcnt32 was not executed due to lack of required 
prereqs. This of course is *the* interesting testcase here.

The reason it's not executed is because tests/x86_amd64_features.c has this:

    if (require_amd && !vendorStringEquals("AuthenticAMD"))
       return FEATURE_NOT_PRESENT;

My laptop identifies itself as "GenuineIntel". Therefore I propose the regtested 
patch below. Anybody sees a problem with that?

Thanks,
    Florian


diff --git a/tests/x86_amd64_features.c b/tests/x86_amd64_features.c
index 488f155b6..5a95e06df 100644
--- a/tests/x86_amd64_features.c
+++ b/tests/x86_amd64_features.c
@@ -148,7 +148,8 @@ static Bool go(char* cpu)
     assert( !(cmask != 0 && dmask != 0 && bmask != 0) );
     assert( !(cmask == 0 && dmask == 0 && bmask == 0) );

-   if (require_amd && !vendorStringEquals("AuthenticAMD"))
+   if (require_amd && !vendorStringEquals("AuthenticAMD") &&
+       !vendorStringEquals("GenuineIntel"))
        return FEATURE_NOT_PRESENT;
        // regardless of what that feature actually is