valgrind-developers — Discussion list for developers of valgrind

Re: [Valgrind-developers] XML summaries - context counts

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

Hi Paul,

On Sun, May 11, 2025 at 01:18:18PM +0200, Paul Floyd via Valgrind-developers wrote:
> I'm about ready to push the changes for
> https://bugs.kde.org/show_bug.cgi?id=390310
> 
> One thing I'd like some other opinion(s) on is the tag used for the
> summaries.
> 
> At the moment the non-xml output looks like
> 
> ==3166== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 3 from 3)
> 
> With this patch the corresponding xml summary is
> 
> 
> <error_summary>
>  <errors>1</errors>
>  <from>1</from>
>  <suppressed>3</suppressed>
>  <suppressed_from>3</suppressed_from>
> </error_summary>
> 
> Do you think that would be clearer using "contexts" rather than "from"?
> 
> 
> <error_summary>
>  <errors>1</errors>
>  <error_contexts>1</error_contexts>
>  <suppressed>3</suppressed>
>  <suppressed_contexts>3</suppressed_contexts>
> </error_summary>

Yes, I think using "contexts" is clearer than using "from".

We should probably also update:

./docs/internals/xml-output.txt
./docs/internals/xml-output-protocol4.txt
./docs/internals/xml-output-protocol5.txt

4 is the default output, 5 is with --track-fds enabled (so it includes
core error elements). Maybe after this change goes in we should update
the (default) protocol version to 6?

Cheers,

Mark

[Valgrind-developers] XML summaries - context counts

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

Hi all

I'm about ready to push the changes for 
https://bugs.kde.org/show_bug.cgi?id=390310

One thing I'd like some other opinion(s) on is the tag used for the 
summaries.

At the moment the non-xml output looks like

==3166== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 3 from 3)

With this patch the corresponding xml summary is


<error_summary>
  <errors>1</errors>
  <from>1</from>
  <suppressed>3</suppressed>
  <suppressed_from>3</suppressed_from>
</error_summary>

Do you think that would be clearer using "contexts" rather than "from"?


<error_summary>
  <errors>1</errors>
  <error_contexts>1</error_contexts>
  <suppressed>3</suppressed>
  <suppressed_contexts>3</suppressed_contexts>
</error_summary>


A+

Paul

Re: [Valgrind-developers] missing -Wformat warnings

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

On 08/05/2025 11:31, Florian Krohm wrote:
>
> I am wondering whether adding -nodefaultlibs to the compile flags helps.
>
> <quote>
> '-nodefaultlibs'
>      Do not use the standard system libraries when linking.  ...
> </quote>
>
> If GCC still adds a dependency on a library function that I'm telling 
> it won't be there - that would be quite odd...

Unfortunately it _is_ a bit odd

https://godbolt.org/z/86nrE4odb

> Could you give that a try? Or may be change VG_(strlen) to return 
> __builtin_strlen(str) ?
>
__builtin_strlen() isn't builtin either, it results in a call to strlen().

LLVM has __attribute__((no_builtin("strlen"))) but not GCC so we can't 
use that.

The only alternative I have for the moment is to drop the optimization 
level to 01 as in the godbolt link above. There are 4 places in the code 
where I had to do that (3 strlen replacements and 
drd_pre_mem_read_asciiz which is effectively doing an strlen as a check 
before calling DRD_(trace_load)).

A+

Paul

[Valgrind-developers] [valgrind] Linux PPC64 syscall: add sys_io_pgetevents

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=01f66db19fd91bbe869d1272ab67e441396cabe5

commit 01f66db19fd91bbe869d1272ab67e441396cabe5
Author: Paul Floyd <pj...@wa...>
Date:   Sun May 11 10:28:01 2025 +0200

    Linux PPC64 syscall: add sys_io_pgetevents

Diff:
---
 coregrind/m_syswrap/syswrap-ppc64-linux.c | 1 +
 include/vki/vki-scnums-ppc64-linux.h      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/coregrind/m_syswrap/syswrap-ppc64-linux.c b/coregrind/m_syswrap/syswrap-ppc64-linux.c
index 7a79c6dee3..007fa6336c 100644
--- a/coregrind/m_syswrap/syswrap-ppc64-linux.c
+++ b/coregrind/m_syswrap/syswrap-ppc64-linux.c
@@ -1022,6 +1022,7 @@ static SyscallTableEntry syscall_table[] = {
    LINXY(__NR_statx,             sys_statx),            // 383
 
    GENX_(__NR_rseq,              sys_ni_syscall),       // 387
+   LINX_(__NR_io_pgetevents,     sys_io_pgetevents),    // 388
 
    LINXY(__NR_io_uring_setup,    sys_io_uring_setup),    // 425
    LINXY(__NR_io_uring_enter,    sys_io_uring_enter),    // 426
diff --git a/include/vki/vki-scnums-ppc64-linux.h b/include/vki/vki-scnums-ppc64-linux.h
index a76fa6d322..6d8b2b508c 100644
--- a/include/vki/vki-scnums-ppc64-linux.h
+++ b/include/vki/vki-scnums-ppc64-linux.h
@@ -408,6 +408,7 @@
 #define __NR_pkey_free		385
 #define __NR_pkey_mprotect	386
 #define __NR_rseq		387
+#define __NR_io_pgetevents	388
 
 #endif /* __VKI_SCNUMS_PPC64_LINUX_H */

[Valgrind-developers] Proposed VALGRIND_TRY_OPTS patch

[D. J. B. <dj...@cr...>]

This patch adds support for VALGRIND_TRY_OPTS that's like VALGRIND_OPTS
but skips unsupported options.

VALGRIND_TRY_OPTS is always after VALGRIND_OPTS. For more fine-grained
control over order, there's also a new --try that skips the next option
if the next option is unsupported. If support for --try is added to the
forthcoming 3.26 via this patch, and then support for some specific --X
is added in (say) valgrind 3.30 or valgrind 2027.10, then scripts can
use --try --X to opportunistically enable --X without breaking 3.26,
3.27, 3.28, and 3.29. Of course, setting --X in VALGRIND_TRY_OPTS has
the big advantage of also not breaking 3.25, 3.24, etc. Anyway,
VALGRIND_TRY_OPTS is implemented internally on top of --try.

The patch adds user-facing documentation of VALGRIND_TRY_OPTS and --try
in docs/xml/manual-core.xml and in valgrind --help. This patch is on top
of HEAD, and adds 18 tests for VALGRIND_TRY_OPTS and --try, while also
tweaking a few existing tests to reflect the changes to --help. A full
"make regtest" under Debian 12 on a Zen 2 shows no new failures compared
to HEAD.

The existing documentation of default options was missing a statement
that default options don't work for pre-early options, and a statement
that default options are disabled by --command-line-only=yes. This patch
includes the missing documentation. This is conceptually separate from
the rest of the patch, but I think anyone reviewing the code+tests for
the rest of the patch will naturally end up bumping into these issues.

---D. J. Bernstein

[Valgrind-developers] Created VALGRIND_3_25_BRANCH

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

Hi developers and packagers,

I created a VALGRIND_3_25_BRANCH where I hope to put commits
which I think distros should pick up. The idea would be to first
commit to the main branch. Then git cherry-pick -x the commit to the
branch (updating NEWS to list all bugs fixed on the branch).

The patches I put on the VALGRIND_3_25_BRANCH are to fix the
close_range syscall, a fixlet for the mount syscall type param and a
workaround for missing riscv_hwprobe syscall support.

I saw multiple reports about users having issues with the close_range
syscall being broken. So it might be good to do a 3.25.1 release from
the branch. I hope distros will pick up these fixes, I have already
added them to the Fedora package. But having an explicit release might
trigger more distros to update. Lets discuss next week if others feel
it is urgent enough.

Please let me know if you find other fixes on trunk you believe really
should be backported to the 3.25 branch/3.25.1 release.

Thanks,

Mark

  Set version to 3.25.1.GIT
  Prepare NEWS for branch 3.25 fixes
  FreeBSD close_range syscall
  Bug 503641 - close_range syscalls started failing with 3.25.0
  mount syscall param filesystemtype may be NULL
  Add workaround for missing riscv_hwprobe syscall (258)

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] Add workaround for missing riscv_hwprobe syscall (258)

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=9b967eacdfcfaf7a6dfa472dc462bc54653a2fb7

commit 9b967eacdfcfaf7a6dfa472dc462bc54653a2fb7
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 13:46:44 2025 +0200

    Add workaround for missing riscv_hwprobe syscall (258)
    
    On riscv newer glibc (2.41) will probe instruction support using the
    riscv_hwprobe syscall. Since Valgrind currently doesn't have a wrapper
    for riscv_hwprobe that causes a Warning. Since the RISC-V Hardware
    Probing Interface is non-trivial and we don't really implement
    extended riscv instructions anyway work around that by "implementing"
    riscv_hwprobe as sys_ni_syscall so it generates an ENOSYS and glibc
    will silently fall back to not using any extended instructions.
    
    https://docs.kernel.org/arch/riscv/hwprobe.html
    
    https://bugs.kde.org/show_bug.cgi?id=503253
    
    (cherry picked from commit 5efdbbd321e6816ab2ae436d38c20bdcc72b4c17)

Diff:
---
 coregrind/m_syswrap/syswrap-riscv64-linux.c | 1 +
 include/vki/vki-scnums-riscv64-linux.h      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/coregrind/m_syswrap/syswrap-riscv64-linux.c b/coregrind/m_syswrap/syswrap-riscv64-linux.c
index e0146f2b0d..ebf9c31053 100644
--- a/coregrind/m_syswrap/syswrap-riscv64-linux.c
+++ b/coregrind/m_syswrap/syswrap-riscv64-linux.c
@@ -545,6 +545,7 @@ static SyscallTableEntry syscall_main_table[] = {
    LINXY(__NR_perf_event_open, sys_perf_event_open),               /* 241 */
    LINXY(__NR_accept4, sys_accept4),                               /* 242 */
    LINXY(__NR_recvmmsg, sys_recvmmsg),                             /* 243 */
+   GENX_(__NR_riscv_hwprobe, sys_ni_syscall),                      /* 258 */
    PLAX_(__NR_riscv_flush_icache, sys_riscv_flush_icache),         /* 259 */
    GENXY(__NR_wait4, sys_wait4),                                   /* 260 */
    LINXY(__NR_prlimit64, sys_prlimit64),                           /* 261 */
diff --git a/include/vki/vki-scnums-riscv64-linux.h b/include/vki/vki-scnums-riscv64-linux.h
index e4cc04a608..17b6839f8a 100644
--- a/include/vki/vki-scnums-riscv64-linux.h
+++ b/include/vki/vki-scnums-riscv64-linux.h
@@ -321,6 +321,7 @@
 #define __NR_mmap __NR3264_mmap
 #define __NR_fadvise64 __NR3264_fadvise64
 
+#define __NR_riscv_hwprobe (__NR_arch_specific_syscall + 14)
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
 
 #endif /* __VKI_SCNUMS_RISCV64_LINUX_H */

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] mount syscall param filesystemtype may be NULL

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=5c943affd2012a9f54a82ce98a6e7f6a5830c3d6

commit 5c943affd2012a9f54a82ce98a6e7f6a5830c3d6
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 00:21:25 2025 +0200

    mount syscall param filesystemtype may be NULL
    
    On Linux the mount syscall, depending on flags provided, the source,
    type and data my be ignored.  We already don't check data and allow
    source to be NULL.  Normally when type is ignored an application will
    provide an empty string "".  But sometimes NULL is passed (like for
    source).  So we now also allow type to be NULL to prevent false
    positives.
    
    Adjust the linux/scalar.c tests so the type param is still
    unaddressable.
    
    https://bugs.kde.org/show_bug.cgi?id=503914
    
    (cherry picked from commit ff6e14ab798af0628c54c6a704c1cb8844a79419)

Diff:
---
 NEWS                                | 1 +
 coregrind/m_syswrap/syswrap-linux.c | 6 ++++--
 memcheck/tests/arm64-linux/scalar.c | 2 +-
 memcheck/tests/x86-linux/scalar.c   | 2 +-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 7cbe6344ac..3f084a52be 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ Branch 3.25
 The following bugs have been fixed or resolved on this branch.
 
 503641  close_range syscalls started failing with 3.25.0
+503914  mount syscall param filesystemtype may be NULL
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 6f3917830f..afd4a618b1 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -1000,7 +1000,8 @@ PRE(sys_mount)
 {
    // Nb: depending on 'flags', the 'type' and 'data' args may be ignored.
    // We are conservative and check everything, except the memory pointed to
-   // by 'data'.
+   // by 'data'. And since both 'source' and 'type' may be ignored, we allow
+   // them to be NULL.
    *flags |= SfMayBlock;
    PRINT("sys_mount( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x(%s), %#"
          FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %#" FMT_REGWORD "x )",
@@ -1012,7 +1013,8 @@ PRE(sys_mount)
    if (ARG1)
       PRE_MEM_RASCIIZ( "mount(source)", ARG1);
    PRE_MEM_RASCIIZ( "mount(target)", ARG2);
-   PRE_MEM_RASCIIZ( "mount(type)", ARG3);
+   if (ARG3)
+      PRE_MEM_RASCIIZ( "mount(type)", ARG3);
 }
 
 PRE(sys_oldumount)
diff --git a/memcheck/tests/arm64-linux/scalar.c b/memcheck/tests/arm64-linux/scalar.c
index 622ea1c47c..49e0ca6a70 100644
--- a/memcheck/tests/arm64-linux/scalar.c
+++ b/memcheck/tests/arm64-linux/scalar.c
@@ -128,7 +128,7 @@ int main(void)
 
    // __NR_mount 21
    GO(__NR_mount, "5s 3m");
-   SY(__NR_mount, x0, x0, x0, x0, x0); FAIL;
+   SY(__NR_mount, x0, x0, x0-1, x0, x0); FAIL;
    
    // __NR_umount arm64 only has umount2
    //GO(__NR_umount, "1s 1m");
diff --git a/memcheck/tests/x86-linux/scalar.c b/memcheck/tests/x86-linux/scalar.c
index 83ed38c4d9..fe36a47ef0 100644
--- a/memcheck/tests/x86-linux/scalar.c
+++ b/memcheck/tests/x86-linux/scalar.c
@@ -137,7 +137,7 @@ int main(void)
 
    // __NR_mount 21
    GO(__NR_mount, "5s 3m");
-   SY(__NR_mount, x0, x0, x0, x0, x0); FAIL;
+   SY(__NR_mount, x0, x0, x0-1, x0, x0); FAIL;
    
    // __NR_umount 22
    GO(__NR_umount, "1s 1m");

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] Bug 503641 - close_range syscalls started failing with 3.25.0

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=4e240e47f7a7560cf9383b6f9483bffd3a5ec289

commit 4e240e47f7a7560cf9383b6f9483bffd3a5ec289
Author: Paul Floyd <pj...@wa...>
Date:   Fri May 2 18:53:09 2025 +0200

    Bug 503641 - close_range syscalls started failing with 3.25.0
    
    (cherry picked from commit 933e542b431f17681cc915de656e853dc16d4fe7)

Diff:
---
 NEWS                                        |  2 +
 coregrind/m_syswrap/syswrap-linux.c         | 58 ++++++++++++++---------------
 memcheck/tests/close_range.stderr.exp.linux |  6 +--
 3 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/NEWS b/NEWS
index 08fc1117d6..7cbe6344ac 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Branch 3.25
 
 The following bugs have been fixed or resolved on this branch.
 
+503641  close_range syscalls started failing with 3.25.0
+
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX
 where XXXXXX is the bug number as listed above.
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index de710c97e4..6f3917830f 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -13699,17 +13699,20 @@ PRE(sys_execveat)
 
 PRE(sys_close_range)
 {
-   SysRes res = VG_(mk_SysRes_Success)(0);
-   Int beg, end;
-   Int first = ARG1;
-   Int last = ARG2;
+   UInt first = ARG1;
+   UInt last = ARG2;
+
+   vg_assert(VG_(log_output_sink).fd == -1 ||
+             VG_(log_output_sink).fd >= VG_(fd_hard_limit));
+   vg_assert(VG_(xml_output_sink).fd == -1 ||
+             VG_(xml_output_sink).fd >= VG_(fd_hard_limit));
 
    FUSE_COMPATIBLE_MAY_BLOCK();
    PRINT("sys_close_range ( %" FMT_REGWORD "u, %" FMT_REGWORD "u, %"
          FMT_REGWORD "u )", ARG1, ARG2, ARG3);
    PRE_REG_READ3(long, "close_range",
                  unsigned int, first, unsigned int, last,
-                 unsigned int, flags);
+                 int, flags);
 
    if (first > last) {
       SET_STATUS_Failure( VKI_EINVAL );
@@ -13724,29 +13727,28 @@ PRE(sys_close_range)
       return;
    }
 
-   beg = end = first;
-   do {
-      if (end > last
-	  || (end == 2/*stderr*/ && VG_(debugLog_getLevel)() > 0)
-	  || end == VG_(log_output_sink).fd
-	  || end == VG_(xml_output_sink).fd) {
-         /* Split the range if it contains a file descriptor we're not
-          * supposed to close. */
-         if (end - 1 >= beg)
-             res = VG_(do_syscall3)(__NR_close_range, (UWord)beg, (UWord)end - 1, ARG3 );
-         beg = end + 1;
+   if (VG_(debugLog_getLevel)() > 0 &&
+      first <= 2U &&
+      last >= 2U &&
+      first != last) {
+      SysRes res = VG_(mk_SysRes_Success)(0);
+      if (first <= 1U) {
+         res = VG_(do_syscall3)(__NR_close_range, first, 1U, ARG3);
       }
-   } while (end++ <= last);
-
-   /* If it failed along the way, it's presumably the flags being wrong. */
-   SET_STATUS_from_SysRes (res);
+      if (!sr_isError(res) && last >= 3U) {
+         res = VG_(do_syscall3)(__NR_close_range, 3U, last, ARG3);
+      }
+      /* If it failed along the way, it's presumably the flags being wrong. */
+      SET_STATUS_from_SysRes (res);
+   } else {
+      SET_STATUS_from_SysRes(VG_(do_syscall3)(__NR_close_range, first, last, ARG3));
+   }
 }
 
 POST(sys_close_range)
 {
-   Int fd;
-   Int first = ARG1;
-   Int last = ARG2;
+   UInt fd;
+   UInt last = ARG2;
 
    if (!VG_(clo_track_fds)
        || (ARG3 & VKI_CLOSE_RANGE_CLOEXEC) != 0)
@@ -13757,13 +13759,11 @@ POST(sys_close_range)
 
    /* If the close_range range is too wide, we don't want to loop
       through the whole range.  */
-   if (last == ~0U)
-     ML_(record_fd_close_range)(tid, first);
+   if (ARG2 >= VG_(fd_hard_limit))
+     ML_(record_fd_close_range)(tid, ARG1);
    else {
-     for (fd = first; fd <= last; fd++)
-       if ((fd != 2/*stderr*/ || VG_(debugLog_getLevel)() == 0)
-           && fd != VG_(log_output_sink).fd
-           && fd != VG_(xml_output_sink).fd)
+     for (fd = ARG1; fd <= last; fd++)
+       if ((fd != 2/*stderr*/ || VG_(debugLog_getLevel)() == 0))
          ML_(record_fd_close)(tid, fd);
    }
 }
diff --git a/memcheck/tests/close_range.stderr.exp.linux b/memcheck/tests/close_range.stderr.exp.linux
index 3a0de7837a..93c4bfa270 100644
--- a/memcheck/tests/close_range.stderr.exp.linux
+++ b/memcheck/tests/close_range.stderr.exp.linux
@@ -1,12 +1,12 @@
 Syscall param close_range(first) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)
 
 Syscall param close_range(last) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)
 
 Syscall param close_range(flags) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] FreeBSD close_range syscall

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=34eb704121e61257ea9f1937ce4e56863d60d793

commit 34eb704121e61257ea9f1937ce4e56863d60d793
Author: Paul Floyd <pj...@wa...>
Date:   Fri May 2 16:59:58 2025 +0200

    FreeBSD close_range syscall
    
    Simplify the checking. No ned to test for reserved fds,
    and just split the close_range if debug output is enabled.
    
    Add a call to close_range with an upper bound of ~0U in the
    testcase.
    
    These changes will probably be the basis for fixing
    https://bugs.kde.org/show_bug.cgi?id=503641
    
    (cherry picked from commit 8edc2fa6f8d70dbe10d634adff2513aa68143a22)

Diff:
---
 coregrind/m_syswrap/syswrap-freebsd.c | 62 +++++++++++++++++------------------
 memcheck/tests/close_range.c          |  5 ++-
 memcheck/tests/close_range.stderr.exp |  6 ++--
 3 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c
index 7ea04fdbc6..41cd075619 100644
--- a/coregrind/m_syswrap/syswrap-freebsd.c
+++ b/coregrind/m_syswrap/syswrap-freebsd.c
@@ -6615,10 +6615,8 @@ POST(sys___realpathat)
 // int close_range(u_int lowfd, u_int highfd, int flags);
 PRE(sys_close_range)
 {
-   SysRes res = VG_(mk_SysRes_Success)(0);
-   unsigned int lowfd = ARG1;
-   unsigned int fd_counter; // will count from lowfd to highfd
-   unsigned int highfd = ARG2;
+   UInt lowfd = ARG1;
+   UInt highfd = ARG2;
 
    /* on linux the may lock if futexes are used
     * there is a lock in the kernel but I assume it's just
@@ -6642,46 +6640,48 @@ PRE(sys_close_range)
       return;
    }
 
-   fd_counter = lowfd;
-   do {
-      if (fd_counter > highfd
-          || (fd_counter == 2U/*stderr*/ && VG_(debugLog_getLevel)() > 0)
-          || fd_counter == VG_(log_output_sink).fd
-          || fd_counter == VG_(xml_output_sink).fd) {
-         /* Split the range if it contains a file descriptor we're not
-          * supposed to close. */
-         if (fd_counter - 1 >= lowfd) {
-            res = VG_(do_syscall3)(__NR_close_range, (UWord)lowfd, (UWord)fd_counter - 1, ARG3 );
-         }
-         lowfd = fd_counter + 1;
-      }
-   } while (fd_counter++ <= highfd);
+   vg_assert(VG_(log_output_sink).fd == -1 ||
+             VG_(log_output_sink).fd >= VG_(fd_hard_limit));
+   vg_assert(VG_(xml_output_sink).fd == -1 ||
+             VG_(xml_output_sink).fd >= VG_(fd_hard_limit));
 
-   /* If it failed along the way, it's presumably the flags being wrong. */
-   SET_STATUS_from_SysRes (res);
+   if (VG_(debugLog_getLevel)() > 0 &&
+      lowfd <= 2U &&
+      highfd >= 2U &&
+      lowfd != highfd) {
+      SysRes res = VG_(mk_SysRes_Success)(0);
+      if (lowfd <= 1U) {
+         res = VG_(do_syscall3)(__NR_close_range, lowfd, 1U, ARG3);
+      }
+      if (!sr_isError(res) && highfd >= 3U) {
+         res = VG_(do_syscall3)(__NR_close_range, 3U, highfd, ARG3);
+      }
+      /* If it failed along the way, it's presumably the flags being wrong. */
+      SET_STATUS_from_SysRes (res);
+   } else {
+      SET_STATUS_from_SysRes(VG_(do_syscall3)(__NR_close_range, lowfd, highfd, ARG3));
+   }
 }
 
 POST(sys_close_range)
 {
-   unsigned int fd;
-   unsigned int last = ARG2;
+   UInt fd;
+   UInt highfd = ARG2;
 
    if (!VG_(clo_track_fds)
        || (ARG3 & VKI_CLOSE_RANGE_CLOEXEC) != 0)
       return;
 
-   if (last >= VG_(fd_hard_limit))
-      last = VG_(fd_hard_limit) - 1;
+   if (highfd >= VG_(fd_hard_limit))
+       highfd = VG_(fd_hard_limit) - 1;
 
    /* If the close_range range is too wide, we don't want to loop
       through the whole range.  */
-   if (ARG2 == ~0U)
-     ML_(record_fd_close_range)(tid, ARG1);
-   else {
-      for (fd = ARG1; fd <= last; fd++)
-         if ((fd != 2/*stderr*/ || VG_(debugLog_getLevel)() == 0)
-             && fd != VG_(log_output_sink).fd
-             && fd != VG_(xml_output_sink).fd)
+   if (ARG2 >= VG_(fd_hard_limit)) {
+      ML_(record_fd_close_range)(tid, ARG1);
+   } else {
+      for (fd = ARG1; fd <= highfd; fd++)
+         if ((fd != 2/*stderr*/ || VG_(debugLog_getLevel)() == 0))
             ML_(record_fd_close)(tid, fd);
    }
 }
diff --git a/memcheck/tests/close_range.c b/memcheck/tests/close_range.c
index 6d8b5a0ba8..5c21e7f42b 100644
--- a/memcheck/tests/close_range.c
+++ b/memcheck/tests/close_range.c
@@ -65,7 +65,7 @@ int main(void)
    
    errno = 0;
    // wrong order
-   close_range(fd3, fd1, 2);
+   close_range(fd3, fd1, 0);
    assert(errno = EINVAL);
    
    errno = 0;
@@ -82,6 +82,9 @@ int main(void)
    
    close_range(3, rl.rlim_cur+1, 0);
    
+   int res = close_range(2U, ~0U, 0);
+   assert(res == 0);
+   
    {
       unsigned a;
       unsigned b;
diff --git a/memcheck/tests/close_range.stderr.exp b/memcheck/tests/close_range.stderr.exp
index 2f4d018f22..51dfcd40a5 100644
--- a/memcheck/tests/close_range.stderr.exp
+++ b/memcheck/tests/close_range.stderr.exp
@@ -1,12 +1,12 @@
 Syscall param close_range(lowfd) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)
 
 Syscall param close_range(highfd) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)
 
 Syscall param close_range(flags) contains uninitialised byte(s)
    at 0x........: close_range (in /...libc...)
-   by 0x........: main (close_range.c:89)
+   by 0x........: main (close_range.c:92)

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] Set version to 3.25.1.GIT

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=f3a76a970d5f0192b4d948c7bea6656b027c9bb0

commit f3a76a970d5f0192b4d948c7bea6656b027c9bb0
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 16:41:27 2025 +0200

    Set version to 3.25.1.GIT

Diff:
---
 configure.ac | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2dfbd1c1af..59eecada29 100755
--- a/configure.ac
+++ b/configure.ac
@@ -17,9 +17,9 @@ AC_PREREQ(2.69)
 # Do not forget to rerun ./autogen.sh
 m4_define([v_major_ver], [3])
 m4_define([v_minor_ver], [25])
-m4_define([v_micro_ver], [0])
-m4_define([v_suffix_ver], [])
-m4_define([v_rel_date], ["25 Apr 2025"])
+m4_define([v_micro_ver], [1])
+m4_define([v_suffix_ver], [GIT])
+m4_define([v_rel_date], ["?? ??? 2025"])
 m4_define([v_version],
 	  m4_if(v_suffix_ver, [],
 		[v_major_ver.v_minor_ver.v_micro_ver],

[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] Prepare NEWS for branch 3.25 fixes

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=1b9e202f13daffe3b4aa0c0e4a5e8e644055145e

commit 1b9e202f13daffe3b4aa0c0e4a5e8e644055145e
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 16:43:06 2025 +0200

    Prepare NEWS for branch 3.25 fixes

Diff:
---
 NEWS | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/NEWS b/NEWS
index cd4670aca6..08fc1117d6 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,15 @@
+Branch 3.25
+~~~~~~~~~~~
+
+* ==================== FIXED BUGS ====================
+
+The following bugs have been fixed or resolved on this branch.
+
+To see details of a given bug, visit
+  https://bugs.kde.org/show_bug.cgi?id=XXXXXX
+where XXXXXX is the bug number as listed above.
+
+
 Release 3.25.0 (25 Apr 2025)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Valgrind-developers] [valgrind] Created branch 'VALGRIND_3_25_BRANCH'

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

The branch 'VALGRIND_3_25_BRANCH' was created pointing to:

 9b967eacdf... Add workaround for missing riscv_hwprobe syscall (258)

Re: [Valgrind-developers] [PATCH] Linux: Inhibit tail calls in cancellable system calls

[Adhemerval Z. N. <adh...@li...>]

On 09/05/25 11:24, Mark Wielaard wrote:
> Hi Adhemerval,
> 
> On Fri, 2025-05-09 at 11:08 -0300, Adhemerval Zanella Netto wrote:
>> On 25/04/25 19:06, Mark Wielaard wrote:
>>> valgrind 3.25.0 have been released and is now in Fedora rawhide and
>>> Fedora 42 with the new glibc syscall_cancel frames. The tail calls on
>>> aarch64 still seem to be a problem for observability of the syscall
>>> call stack.
>>
>> I am trying to check if patch to inline the cancellation wrappers [1]
>> would help it, but I am not sure how exactly would handle stacktraces
>> that should be artificial and only represented for debug information.
>>
>> With the patch applied, both x86_64 and aarch64 should inline the
>> syscall_cancel and internal_syscall_cancel call, only required an
>> extra __syscall_cancel_arch call for the case when the process it
>> multithread.
>>
>> On x86_64 it does shows as expected:
>>
>> valgrind-git (x86_64)$ ./coregrind/valgrind memcheck/tests/sendmsg
>> [...]
>> ==2131145== Syscall param sendmsg(msg) points to uninitialised byte(s)
>> ==2131145==    at 0x4972EE0: syscall_cancel (sysdep-cancel.h:83)
>> ==2131145==    by 0x4972EE0: sendmsg (sendmsg.c:28)
>> ==2131145==    by 0x4001332: main (sendmsg.c:46)
>> ==2131145==  Address 0x1ffefff850 is on thread 1's stack
>> ==2131145==  in frame #1, created by main (sendmsg.c:13)
>> [...]
>>
>> But on aarch64 it shows internal_syscall_cancel, which is indeed
>> inlined:
>>
>> valgrind-git (aarch64)$ ./coregrind/valgrind memcheck/tests/sendmsg
>> [...]
>> ==483437== Syscall param sendmsg(msg) points to uninitialised byte(s)
>> ==483437==    at 0x49D250C: internal_syscall_cancel (sysdep-cancel.h:44)
>> ==483437==    by 0x49D250C: syscall_cancel (sysdep-cancel.h:79)
>> ==483437==    by 0x49D250C: sendmsg (sendmsg.c:28)
>> ==483437==    by 0x4000B4B: main (sendmsg.c:46)
>> ==483437==  Address 0x1ffefffaf8 is on thread 1's stack
>> ==483437==  in frame #1, created by main (sendmsg.c:13)
>>
>> I am not sure if valgrind consider this an error, nor if it should be
>> valgrind or compiler to handle this correctly.  I am not aware of any
>> attribute if can properly used to 'hide' internal_syscall_cancel in
>> this case, or even if it makes sense.
> 
> Interesting. I would have assumed valgrind would have stripped out the
> syscall_cancel calls. But it looks like we assume they always have a __
> prefix (__syscall_cancel_arch, __internal_syscall_cancel and
> __syscall_cancel). Apparently with glibc debuginfo installed it picks
> up the inlined DWARF name. But we can filter those out too.
> 
> I think the above would work without glibc debuginfo installed because
> then valgrind won't even know/see the inlined names.

I have tested only with a unstriped glibc build, so I am not sure how 
it would play with debuginfo.

> 
> Just to be sure, you are using the current git version of valgrind?
> And you haven't explicitly installed that version? If so, could you try
> the above with:
> 
>  ./vg-in-place memcheck/tests/sendmsg
> 
> vg-in-place makes sure the just compiled tools are used.

I took care to export VALGRIND_LIB to the proper location, but the results
with vg-in-place are the same.

> 
> Thanks,
> 
> Mark

Re: [Valgrind-developers] [PATCH] Linux: Inhibit tail calls in cancellable system calls

[Adhemerval Z. N. <adh...@li...>]

On 25/04/25 19:06, Mark Wielaard wrote:
> Hi all,
> 
> On Fri, Apr 18, 2025 at 05:46:54PM +0200, Mark Wielaard wrote:
>> On Mon, 2025-04-14 at 14:06 +0200, Mark Wielaard wrote:
>>> On Sun, Apr 06, 2025 at 03:23:54PM +0200, Mark Wielaard wrote:
>>>> On Mon, Mar 31, 2025 at 11:29:41AM +0200, Mark Wielaard wrote:
>>>>> On Fri, Mar 28, 2025 at 07:02:28PM +0100, Mark Wielaard wrote:
>>>>>> On Fri, 2025-03-21 at 14:01 +0100, Florian Weimer wrote:
>>>>>>> Without this change, the system call wrapper function is not visible
>>>>>>> on the stack at the time of the system call, which causes problems
>>>>>>> for interception tools such as valgrind.
>>>>>>>
>>>>>>> Enhances commit 89b53077d2a58f00e7debdfe58afabe953dac60d ("nptl: Fix
>>>>>>> Race conditions in pthread cancellation [BZ#12683]").
>>>>>>>
>>>>>>> Tested on i686-linux-gnu, powerpc64le-linux-gnu, x86_64-linux-gnu.
>>>>>>> (We're still discussing if valgrind needs this, but if it does, here's a
>>>>>>> patch.)
>>>>>>
>>>>>> I implemented the valgrind part of skipping the syscall_cancel frames
>>>>>> here: https://bugs.kde.org/show_bug.cgi?id=502126#c2
>>>>>> And there is a valgrind package build for fedora rawhide:
>>>>>> https://koji.fedoraproject.org/koji/buildinfo?buildID=2687393
>>>>>>
>>>>>> For ppc64le, s390x and x86_64 that patch seems enough.
>>>>>>
>>>>>> For i686 and aarch64 there does seem to be an issue with missing the
>>>>>> glibc calling function because of a tail call.
>>>>>>
>>>>>> Also on i686 there is another extra frame on top __libc_do_syscall.
>>>>>
>>>>> I extended the patch to cover some extra sycall wrapper function
>>>>> symbols on i386 and armhf and pushed it to valgrind trunk and
>>>>> VALGRIND_3_24_BRANCH. There are builds for fedora rawhide and
>>>>> f42. This does seem to show that only on arm64 the tail calls
>>>>> obscure observing the full call stack.
>>>>
>>>> This has now landed in fedora rawhide and f42. Test results look good,
>>>> except for some if the arm64 tests where the tail calls obscure
>>>> observing the full call stack. Please let me know if you need any more
>>>> input from us to get this fix in glibc.
>>>
>>> Please let me know. Valgrind test results for syscall backtraces on
>>> anything except arm64 look good. We are working on valgrind 3.25.0
>>> now, to be released around April 24.
>>
>> valgrind 3.25.0-RC1 has been released and test results look good on
>> most arches. arm64 does show the issue described above where the tail
>> calls obscure observing the full call stack when doing system calls.
> 
> valgrind 3.25.0 have been released and is now in Fedora rawhide and
> Fedora 42 with the new glibc syscall_cancel frames. The tail calls on
> aarch64 still seem to be a problem for observability of the syscall
> call stack.
> 

I am trying to check if patch to inline the cancellation wrappers [1]
would help it, but I am not sure how exactly would handle stacktraces
that should be artificial and only represented for debug information.

With the patch applied, both x86_64 and aarch64 should inline the
syscall_cancel and internal_syscall_cancel call, only required an
extra __syscall_cancel_arch call for the case when the process it
multithread.

On x86_64 it does shows as expected:

valgrind-git (x86_64)$ ./coregrind/valgrind memcheck/tests/sendmsg
[...]
==2131145== Syscall param sendmsg(msg) points to uninitialised byte(s)
==2131145==    at 0x4972EE0: syscall_cancel (sysdep-cancel.h:83)
==2131145==    by 0x4972EE0: sendmsg (sendmsg.c:28)
==2131145==    by 0x4001332: main (sendmsg.c:46)
==2131145==  Address 0x1ffefff850 is on thread 1's stack
==2131145==  in frame #1, created by main (sendmsg.c:13)
[...]

But on aarch64 it shows internal_syscall_cancel, which is indeed
inlined:

valgrind-git (aarch64)$ ./coregrind/valgrind memcheck/tests/sendmsg
[...]
==483437== Syscall param sendmsg(msg) points to uninitialised byte(s)
==483437==    at 0x49D250C: internal_syscall_cancel (sysdep-cancel.h:44)
==483437==    by 0x49D250C: syscall_cancel (sysdep-cancel.h:79)
==483437==    by 0x49D250C: sendmsg (sendmsg.c:28)
==483437==    by 0x4000B4B: main (sendmsg.c:46)
==483437==  Address 0x1ffefffaf8 is on thread 1's stack
==483437==  in frame #1, created by main (sendmsg.c:13)

I am not sure if valgrind consider this an error, nor if it should be
valgrind or compiler to handle this correctly.  I am not aware of any
attribute if can properly used to 'hide' internal_syscall_cancel in
this case, or even if it makes sense.

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/cancel-wrappers-inline

Re: [Valgrind-developers] [PATCH] Linux: Inhibit tail calls in cancellable system calls

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

Hi Adhemerval,

On Fri, 2025-05-09 at 11:08 -0300, Adhemerval Zanella Netto wrote:
> On 25/04/25 19:06, Mark Wielaard wrote:
> > valgrind 3.25.0 have been released and is now in Fedora rawhide and
> > Fedora 42 with the new glibc syscall_cancel frames. The tail calls on
> > aarch64 still seem to be a problem for observability of the syscall
> > call stack.
> 
> I am trying to check if patch to inline the cancellation wrappers [1]
> would help it, but I am not sure how exactly would handle stacktraces
> that should be artificial and only represented for debug information.
> 
> With the patch applied, both x86_64 and aarch64 should inline the
> syscall_cancel and internal_syscall_cancel call, only required an
> extra __syscall_cancel_arch call for the case when the process it
> multithread.
> 
> On x86_64 it does shows as expected:
> 
> valgrind-git (x86_64)$ ./coregrind/valgrind memcheck/tests/sendmsg
> [...]
> ==2131145== Syscall param sendmsg(msg) points to uninitialised byte(s)
> ==2131145==    at 0x4972EE0: syscall_cancel (sysdep-cancel.h:83)
> ==2131145==    by 0x4972EE0: sendmsg (sendmsg.c:28)
> ==2131145==    by 0x4001332: main (sendmsg.c:46)
> ==2131145==  Address 0x1ffefff850 is on thread 1's stack
> ==2131145==  in frame #1, created by main (sendmsg.c:13)
> [...]
> 
> But on aarch64 it shows internal_syscall_cancel, which is indeed
> inlined:
> 
> valgrind-git (aarch64)$ ./coregrind/valgrind memcheck/tests/sendmsg
> [...]
> ==483437== Syscall param sendmsg(msg) points to uninitialised byte(s)
> ==483437==    at 0x49D250C: internal_syscall_cancel (sysdep-cancel.h:44)
> ==483437==    by 0x49D250C: syscall_cancel (sysdep-cancel.h:79)
> ==483437==    by 0x49D250C: sendmsg (sendmsg.c:28)
> ==483437==    by 0x4000B4B: main (sendmsg.c:46)
> ==483437==  Address 0x1ffefffaf8 is on thread 1's stack
> ==483437==  in frame #1, created by main (sendmsg.c:13)
> 
> I am not sure if valgrind consider this an error, nor if it should be
> valgrind or compiler to handle this correctly.  I am not aware of any
> attribute if can properly used to 'hide' internal_syscall_cancel in
> this case, or even if it makes sense.

Interesting. I would have assumed valgrind would have stripped out the
syscall_cancel calls. But it looks like we assume they always have a __
prefix (__syscall_cancel_arch, __internal_syscall_cancel and
__syscall_cancel). Apparently with glibc debuginfo installed it picks
up the inlined DWARF name. But we can filter those out too.

I think the above would work without glibc debuginfo installed because
then valgrind won't even know/see the inlined names.

Just to be sure, you are using the current git version of valgrind?
And you haven't explicitly installed that version? If so, could you try
the above with:

 ./vg-in-place memcheck/tests/sendmsg

vg-in-place makes sure the just compiled tools are used.

Thanks,

Mark

[Valgrind-developers] [valgrind] riscv64: Fix nan-boxing for single-precision calculations

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=9dd24c9b57cde064ca8b356c985b2e1cb7972adc

commit 9dd24c9b57cde064ca8b356c985b2e1cb7972adc
Author: Ivan Tetyushkin <iva...@sy...>
Date:   Mon Apr 21 11:59:48 2025 +0300

    riscv64: Fix nan-boxing for single-precision calculations
    
    For float values, for arithmetics we expect to have
    canonical nan if used double register is not currectly nan-boxed.
    
    https://bugs.kde.org/show_bug.cgi?id=503098

Diff:
---
 NEWS                                  |  1 +
 VEX/priv/guest_riscv64_toIR.c         | 28 +++++++++++++++++++++-------
 none/tests/riscv64/float32.c          |  6 ++++++
 none/tests/riscv64/float32.stdout.exp |  3 +++
 4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 1ced90caef..460eb56569 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather
 than mailing the developers (or mailing lists) directly -- bugs that
 are not entered into bugzilla tend to get forgotten about or ignored.
 
+503098  Incorrect NAN-boxing for float registers in RISC-V
 503641  close_range syscalls started failing with 3.25.0
 503677  duplicated-cond compiler warning in dis_RV64M
 503817  s390x: fix 'ordered comparison of pointer with integer zero' compiler warnings
diff --git a/VEX/priv/guest_riscv64_toIR.c b/VEX/priv/guest_riscv64_toIR.c
index 393a7ca7d0..59297acd15 100644
--- a/VEX/priv/guest_riscv64_toIR.c
+++ b/VEX/priv/guest_riscv64_toIR.c
@@ -522,9 +522,13 @@ static IRExpr* getFReg32(UInt fregNo)
    vassert(fregNo < 32);
    /* Note that the following access depends on the host being little-endian
       which is checked in disInstr_RISCV64(). */
-   /* TODO Check that the value is correctly NaN-boxed. If not then return
-      the 32-bit canonical qNaN, as mandated by the RISC-V ISA. */
-   return IRExpr_Get(offsetFReg(fregNo), Ity_F32);
+   IRExpr* f64       = getFReg64(fregNo);
+   IRExpr* high_half = unop(Iop_64HIto32, unop(Iop_ReinterpF64asI64, f64));
+   IRExpr* cond      = binop(Iop_CmpEQ32, high_half, mkU32(0xffffffff));
+   IRExpr* res       = IRExpr_ITE(
+      cond, IRExpr_Get(offsetFReg(fregNo), Ity_F32),
+      /* canonical nan */ unop(Iop_ReinterpI32asF32, mkU32(0x7fc00000)));
+   return res;
 }
 
 /* Write a 32-bit value into a guest floating-point register. */
@@ -2160,8 +2164,10 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres,
       UInt  rs2     = INSN(24, 20);
       UInt  imm11_0 = INSN(31, 25) << 5 | INSN(11, 7);
       ULong simm    = vex_sx_to_64(imm11_0, 12);
-      storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)),
-              getFReg32(rs2));
+      // do not modify the bits being transferred;
+      IRExpr* f64 = getFReg64(rs2);
+      IRExpr* i32 = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, f64));
+      storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)), i32);
       DIP("fsw %s, %lld(%s)\n", nameFReg(rs2), (Long)simm, nameIReg(rs1));
       return True;
    }
@@ -2456,8 +2462,16 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres,
        INSN(24, 20) == 0b00000 && INSN(31, 25) == 0b1110000) {
       UInt rd  = INSN(11, 7);
       UInt rs1 = INSN(19, 15);
-      if (rd != 0)
-         putIReg32(irsb, rd, unop(Iop_ReinterpF32asI32, getFReg32(rs1)));
+      if (rd != 0) {
+         // For RV64, the higher 32 bits of the destination register are filled
+         // with copies of the floating-point number’s sign bit.
+         IRExpr* freg      = getFReg64(rs1);
+         IRExpr* low_half  = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, freg));
+         IRExpr* sign      = binop(Iop_And32, low_half, mkU32(1u << 31));
+         IRExpr* cond      = binop(Iop_CmpEQ32, sign, mkU32(1u << 31));
+         IRExpr* high_part = IRExpr_ITE(cond, mkU32(0xffffffff), mkU32(0));
+         putIReg64(irsb, rd, binop(Iop_32HLto64, high_part, low_half));
+      }
       DIP("fmv.x.w %s, %s\n", nameIReg(rd), nameFReg(rs1));
       return True;
    }
diff --git a/none/tests/riscv64/float32.c b/none/tests/riscv64/float32.c
index b63305a64e..f635bc7614 100644
--- a/none/tests/riscv64/float32.c
+++ b/none/tests/riscv64/float32.c
@@ -1578,6 +1578,12 @@ static void test_float32_additions(void)
    TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x60, fa0, a0);
    /* 2**24+1 (DYN-RMM) -> 2**24+2 (NX) */
    TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x80, fa0, a0);
+
+   // check nan-boxing
+   /* fabs.s rd, rs1 */
+   TESTINST_1_2_F(4, "fsgnjx.s fa0, fa1, fa1", 0xfaffffff3f800000,
+                  0xfaffffff3f800000, 0x00, fa0, fa1, fa1);
+
 }
 
 int main(void)
diff --git a/none/tests/riscv64/float32.stdout.exp b/none/tests/riscv64/float32.stdout.exp
index 013c7eda21..734370d518 100644
--- a/none/tests/riscv64/float32.stdout.exp
+++ b/none/tests/riscv64/float32.stdout.exp
@@ -1554,3 +1554,6 @@ fcvt.s.lu fa0, a0 ::
 fcvt.s.lu fa0, a0 ::
   inputs: a0=0x0000000001000001, fcsr=0x00000080
   output: fa0=0xffffffff4b800001, fcsr=0x00000081
+fsgnjx.s fa0, fa1, fa1 ::
+  inputs: fa1=0xfaffffff3f800000, fa1=0xfaffffff3f800000, fcsr=0x00000000
+  output: fa0=0xffffffff7fc00000, fcsr=0x00000000

[Valgrind-developers] [valgrind] Add workaround for missing riscv_hwprobe syscall (258)

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=5efdbbd321e6816ab2ae436d38c20bdcc72b4c17

commit 5efdbbd321e6816ab2ae436d38c20bdcc72b4c17
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 13:46:44 2025 +0200

    Add workaround for missing riscv_hwprobe syscall (258)
    
    On riscv newer glibc (2.41) will probe instruction support using the
    riscv_hwprobe syscall. Since Valgrind currently doesn't have a wrapper
    for riscv_hwprobe that causes a Warning. Since the RISC-V Hardware
    Probing Interface is non-trivial and we don't really implement
    extended riscv instructions anyway work around that by "implementing"
    riscv_hwprobe as sys_ni_syscall so it generates an ENOSYS and glibc
    will silently fall back to not using any extended instructions.
    
    https://docs.kernel.org/arch/riscv/hwprobe.html
    
    https://bugs.kde.org/show_bug.cgi?id=503253

Diff:
---
 coregrind/m_syswrap/syswrap-riscv64-linux.c | 1 +
 include/vki/vki-scnums-riscv64-linux.h      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/coregrind/m_syswrap/syswrap-riscv64-linux.c b/coregrind/m_syswrap/syswrap-riscv64-linux.c
index e0146f2b0d..ebf9c31053 100644
--- a/coregrind/m_syswrap/syswrap-riscv64-linux.c
+++ b/coregrind/m_syswrap/syswrap-riscv64-linux.c
@@ -545,6 +545,7 @@ static SyscallTableEntry syscall_main_table[] = {
    LINXY(__NR_perf_event_open, sys_perf_event_open),               /* 241 */
    LINXY(__NR_accept4, sys_accept4),                               /* 242 */
    LINXY(__NR_recvmmsg, sys_recvmmsg),                             /* 243 */
+   GENX_(__NR_riscv_hwprobe, sys_ni_syscall),                      /* 258 */
    PLAX_(__NR_riscv_flush_icache, sys_riscv_flush_icache),         /* 259 */
    GENXY(__NR_wait4, sys_wait4),                                   /* 260 */
    LINXY(__NR_prlimit64, sys_prlimit64),                           /* 261 */
diff --git a/include/vki/vki-scnums-riscv64-linux.h b/include/vki/vki-scnums-riscv64-linux.h
index e4cc04a608..17b6839f8a 100644
--- a/include/vki/vki-scnums-riscv64-linux.h
+++ b/include/vki/vki-scnums-riscv64-linux.h
@@ -321,6 +321,7 @@
 #define __NR_mmap __NR3264_mmap
 #define __NR_fadvise64 __NR3264_fadvise64
 
+#define __NR_riscv_hwprobe (__NR_arch_specific_syscall + 14)
 #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
 
 #endif /* __VKI_SCNUMS_RISCV64_LINUX_H */

Re: [Valgrind-developers] [PATCH v2] lib: tst_test: Add reproducible output.

[Martin D. <md...@su...>]

Hi,
Reviewed-by: Martin Doucha <md...@su...>

On 09. 05. 25 11:28, Cyril Hrubis wrote:
> This commit adds an environment variable LTP_REPRODUCIBLE_OUTPUT that
> when set skips printing parts of the test messages that may contain data
> that differ on subsequent runs (e.g. pids).
> 
> With this you can run a test twice under a different conditions and
> check if the test codeflow was identical by simply doing diff of the
> outputs from the two runs.
> 
> Signed-off-by: Cyril Hrubis <ch...@su...>
> Suggested-by: Martin Doucha <md...@su...>
> CC: val...@li...
> ---
>   lib/tst_test.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2bb4519dd..f14627544 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -64,6 +64,7 @@ static int mntpoint_mounted;
>   static int ovl_mounted;
>   static struct timespec tst_start_time; /* valid only for test pid */
>   static int tdebug;
> +static int reproducible_output;
>   
>   struct results {
>   	int passed;
> @@ -312,6 +313,9 @@ static void print_result(const char *file, const int lineno, int ttype,
>   	str += ret;
>   	size -= ret;
>   
> +	if (reproducible_output)
> +		goto print;
> +
>   	ssize = size - 2;
>   	ret = vsnprintf(str, size, fmt, va);
>   	str += MIN(ret, ssize);
> @@ -329,6 +333,7 @@ static void print_result(const char *file, const int lineno, int ttype,
>   				"Next message is too long and truncated:");
>   	}
>   
> +print:
>   	snprintf(str, size, "\n");
>   
>   	/* we might be called from signal handler, so use write() */
> @@ -1312,6 +1317,8 @@ static void do_setup(int argc, char *argv[])
>   	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>   		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
>   
> +	reproducible_output = !!getenv("LTP_REPRODUCIBLE_OUTPUT");
> +
>   	assert_test_fn();
>   
>   	TCID = tid = get_tid(argv);


-- 
Martin Doucha   md...@su...
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[Valgrind-developers] [PATCH v2] lib: tst_test: Add reproducible output.

[Cyril H. <ch...@su...>]

This commit adds an environment variable LTP_REPRODUCIBLE_OUTPUT that
when set skips printing parts of the test messages that may contain data
that differ on subsequent runs (e.g. pids).

With this you can run a test twice under a different conditions and
check if the test codeflow was identical by simply doing diff of the
outputs from the two runs.

Signed-off-by: Cyril Hrubis <ch...@su...>
Suggested-by: Martin Doucha <md...@su...>
CC: val...@li...
---
 lib/tst_test.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2bb4519dd..f14627544 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -64,6 +64,7 @@ static int mntpoint_mounted;
 static int ovl_mounted;
 static struct timespec tst_start_time; /* valid only for test pid */
 static int tdebug;
+static int reproducible_output;
 
 struct results {
 	int passed;
@@ -312,6 +313,9 @@ static void print_result(const char *file, const int lineno, int ttype,
 	str += ret;
 	size -= ret;
 
+	if (reproducible_output)
+		goto print;
+
 	ssize = size - 2;
 	ret = vsnprintf(str, size, fmt, va);
 	str += MIN(ret, ssize);
@@ -329,6 +333,7 @@ static void print_result(const char *file, const int lineno, int ttype,
 				"Next message is too long and truncated:");
 	}
 
+print:
 	snprintf(str, size, "\n");
 
 	/* we might be called from signal handler, so use write() */
@@ -1312,6 +1317,8 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
 		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
 
+	reproducible_output = !!getenv("LTP_REPRODUCIBLE_OUTPUT");
+
 	assert_test_fn();
 
 	TCID = tid = get_tid(argv);
-- 
2.49.0

Re: [Valgrind-developers] [PATCH] lib: tst_test: Add reproducible output.

[Cyril H. <ch...@su...>]

Hi!
> >   struct results {
> >   	int passed;
> > @@ -304,6 +305,9 @@ static void print_result(const char *file, const int lineno, int ttype,
> >   	str += ret;
> >   	size -= ret;
> >   
> > +	if (reproducible_output)
> > +		goto print;
> > +
> 
> I'd move this goto one code chunk down (after the color if-else) so that 
> we also get the res string in the output. Or at least add a strncpy(str, 
> res, size) before the goto if you want to bypass the color logic.

The color output is disabled when output is redirected into a file so I
suppose that we can simply move the goto after we print the result. I
will send a v2.

-- 
Cyril Hrubis
ch...@su...

Re: [Valgrind-developers] [PATCH] lib: tst_test: Add reproducible output.

[Martin D. <md...@su...>]

Hi,
one suggestion below.

On 09. 05. 25 10:34, Cyril Hrubis wrote:
> This commit adds an environment variable LTP_REPRODUCIBLE_OUTPUT that
> when set skips printing parts of the test messages that may contain data
> that differ on subsequent runs (e.g. pids).
> 
> With this you can run a test twice under a different conditions and
> check if the test codeflow was identical by simply doing diff of the
> outputs from the two runs.
> 
> Signed-off-by: Cyril Hrubis <ch...@su...>
> Suggested-by: Martin Doucha <md...@su...>
> CC: val...@li...
> ---
>   lib/tst_test.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2bb4519dd..3f94a1607 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -64,6 +64,7 @@ static int mntpoint_mounted;
>   static int ovl_mounted;
>   static struct timespec tst_start_time; /* valid only for test pid */
>   static int tdebug;
> +static int reproducible_output;
>   
>   struct results {
>   	int passed;
> @@ -304,6 +305,9 @@ static void print_result(const char *file, const int lineno, int ttype,
>   	str += ret;
>   	size -= ret;
>   
> +	if (reproducible_output)
> +		goto print;
> +

I'd move this goto one code chunk down (after the color if-else) so that 
we also get the res string in the output. Or at least add a strncpy(str, 
res, size) before the goto if you want to bypass the color logic.

>   	if (tst_color_enabled(STDERR_FILENO))
>   		ret = snprintf(str, size, "%s%s: %s", tst_ttype2color(ttype),
>   			       res, ANSI_COLOR_RESET);
> @@ -329,6 +333,7 @@ static void print_result(const char *file, const int lineno, int ttype,
>   				"Next message is too long and truncated:");
>   	}
>   
> +print:
>   	snprintf(str, size, "\n");
>   
>   	/* we might be called from signal handler, so use write() */
> @@ -1312,6 +1317,8 @@ static void do_setup(int argc, char *argv[])
>   	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>   		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
>   
> +	reproducible_output = !!getenv("LTP_REPRODUCIBLE_OUTPUT");
> +
>   	assert_test_fn();
>   
>   	TCID = tid = get_tid(argv);


-- 
Martin Doucha   md...@su...
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[Valgrind-developers] [PATCH] lib: tst_test: Add reproducible output.

[Cyril H. <ch...@su...>]

This commit adds an environment variable LTP_REPRODUCIBLE_OUTPUT that
when set skips printing parts of the test messages that may contain data
that differ on subsequent runs (e.g. pids).

With this you can run a test twice under a different conditions and
check if the test codeflow was identical by simply doing diff of the
outputs from the two runs.

Signed-off-by: Cyril Hrubis <ch...@su...>
Suggested-by: Martin Doucha <md...@su...>
CC: val...@li...
---
 lib/tst_test.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 2bb4519dd..3f94a1607 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -64,6 +64,7 @@ static int mntpoint_mounted;
 static int ovl_mounted;
 static struct timespec tst_start_time; /* valid only for test pid */
 static int tdebug;
+static int reproducible_output;
 
 struct results {
 	int passed;
@@ -304,6 +305,9 @@ static void print_result(const char *file, const int lineno, int ttype,
 	str += ret;
 	size -= ret;
 
+	if (reproducible_output)
+		goto print;
+
 	if (tst_color_enabled(STDERR_FILENO))
 		ret = snprintf(str, size, "%s%s: %s", tst_ttype2color(ttype),
 			       res, ANSI_COLOR_RESET);
@@ -329,6 +333,7 @@ static void print_result(const char *file, const int lineno, int ttype,
 				"Next message is too long and truncated:");
 	}
 
+print:
 	snprintf(str, size, "\n");
 
 	/* we might be called from signal handler, so use write() */
@@ -1312,6 +1317,8 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
 		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
 
+	reproducible_output = !!getenv("LTP_REPRODUCIBLE_OUTPUT");
+
 	assert_test_fn();
 
 	TCID = tid = get_tid(argv);
-- 
2.49.0

[Valgrind-developers] [valgrind] mount syscall param filesystemtype may be NULL

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

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=ff6e14ab798af0628c54c6a704c1cb8844a79419

commit ff6e14ab798af0628c54c6a704c1cb8844a79419
Author: Mark Wielaard <ma...@kl...>
Date:   Fri May 9 00:21:25 2025 +0200

    mount syscall param filesystemtype may be NULL
    
    On Linux the mount syscall, depending on flags provided, the source,
    type and data my be ignored.  We already don't check data and allow
    source to be NULL.  Normally when type is ignored an application will
    provide an empty string "".  But sometimes NULL is passed (like for
    source).  So we now also allow type to be NULL to prevent false
    positives.
    
    Adjust the linux/scalar.c tests so the type param is still
    unaddressable.
    
    https://bugs.kde.org/show_bug.cgi?id=503914

Diff:
---
 NEWS                                | 1 +
 coregrind/m_syswrap/syswrap-linux.c | 6 ++++--
 memcheck/tests/arm64-linux/scalar.c | 2 +-
 memcheck/tests/x86-linux/scalar.c   | 2 +-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 7c66ec08bb..1ced90caef 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 503641  close_range syscalls started failing with 3.25.0
 503677  duplicated-cond compiler warning in dis_RV64M
 503817  s390x: fix 'ordered comparison of pointer with integer zero' compiler warnings
+503914  mount syscall param filesystemtype may be NULL
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 6f3917830f..afd4a618b1 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -1000,7 +1000,8 @@ PRE(sys_mount)
 {
    // Nb: depending on 'flags', the 'type' and 'data' args may be ignored.
    // We are conservative and check everything, except the memory pointed to
-   // by 'data'.
+   // by 'data'. And since both 'source' and 'type' may be ignored, we allow
+   // them to be NULL.
    *flags |= SfMayBlock;
    PRINT("sys_mount( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x(%s), %#"
          FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %#" FMT_REGWORD "x )",
@@ -1012,7 +1013,8 @@ PRE(sys_mount)
    if (ARG1)
       PRE_MEM_RASCIIZ( "mount(source)", ARG1);
    PRE_MEM_RASCIIZ( "mount(target)", ARG2);
-   PRE_MEM_RASCIIZ( "mount(type)", ARG3);
+   if (ARG3)
+      PRE_MEM_RASCIIZ( "mount(type)", ARG3);
 }
 
 PRE(sys_oldumount)
diff --git a/memcheck/tests/arm64-linux/scalar.c b/memcheck/tests/arm64-linux/scalar.c
index 622ea1c47c..49e0ca6a70 100644
--- a/memcheck/tests/arm64-linux/scalar.c
+++ b/memcheck/tests/arm64-linux/scalar.c
@@ -128,7 +128,7 @@ int main(void)
 
    // __NR_mount 21
    GO(__NR_mount, "5s 3m");
-   SY(__NR_mount, x0, x0, x0, x0, x0); FAIL;
+   SY(__NR_mount, x0, x0, x0-1, x0, x0); FAIL;
    
    // __NR_umount arm64 only has umount2
    //GO(__NR_umount, "1s 1m");
diff --git a/memcheck/tests/x86-linux/scalar.c b/memcheck/tests/x86-linux/scalar.c
index 83ed38c4d9..fe36a47ef0 100644
--- a/memcheck/tests/x86-linux/scalar.c
+++ b/memcheck/tests/x86-linux/scalar.c
@@ -137,7 +137,7 @@ int main(void)
 
    // __NR_mount 21
    GO(__NR_mount, "5s 3m");
-   SY(__NR_mount, x0, x0, x0, x0, x0); FAIL;
+   SY(__NR_mount, x0, x0, x0-1, x0, x0); FAIL;
    
    // __NR_umount 22
    GO(__NR_umount, "1s 1m");

Re: [Valgrind-developers] missing -Wformat warnings

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

On 07.05.25 21:58, Paul Floyd via Valgrind-developers wrote:
>>
>> I noticed that the presence of -fno-builtin suppresses -Wformat warnings.
>> First I thought this to be a GCC bug but it is not.
> 
> Strange. I don't see any relationship between the two.
> 

The -fno-builtin gcc docs are not very precise.. I believe that
they mean C/C++/whatever library functions when they talk about
built-in functions (other than those beginning with __builtin_).

-fno-builtin then means: when you encounter a function that has the same
name as a built-in function, forget everything you know about that function.
That includes functions attributes. And without __attribute__((format...))
being present there will be no -Wformat warnings.

<quote>
For example, warnings are given with '-Wformat' for bad calls to
'printf' when 'printf' is built in.
</quote>


> No. On openSUSE Leap 15.6 amd64 with GCC 14.2 I get link errors. We would need 
> to provide a 'strlen' implementation at least for amd64 Linux.
> 
> Here are the errors:
> 
> ../coregrind/link_tool_exe_linux 0x58000000 gcc-14     -o memcheck-amd64-linux  
> -m64 -O2 -g -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith 
> -Wstrict-prototypes -Wmissing-declarations -Wno-unused-result -Wcast-align 
> -Wcast-qual -Wwrite-strings -Wempty-body -Wformat -Wformat-signedness 
> -Wformat-security -Wignored-qualifiers -Wmissing-parameter-type -Wlogical-op 
> -Wenum-conversion -Wimplicit-fallthrough=2 -Wold-style-declaration 
> -finline-functions -fno-stack-protector -fno-strict-aliasing   
> -fomit-frame-pointer -O2 -static -nodefaultlibs -nostartfiles -u _start  -m64 
> memcheck_amd64_linux-mc_leakcheck.o memcheck_amd64_linux-mc_malloc_wrappers.o 
> memcheck_amd64_linux-mc_main.o memcheck_amd64_linux-mc_main_asm.o 
> memcheck_amd64_linux-mc_translate.o memcheck_amd64_linux-mc_machine.o 
> memcheck_amd64_linux-mc_errors.o ../coregrind/libcoregrind-amd64-linux.a 
> ../VEX/libvex-amd64-linux.a -lgcc ../coregrind/libgcc-sup-amd64-linux.a
> /usr/lib64/gcc/x86_64-suse-linux/14/../../../../x86_64-suse-linux/bin/ld: 
> ../coregrind/libcoregrind-amd64-linux.a(libcoregrind_amd64_linux_a-m_libcbase.o): in function `vgPlain_strlen':
> /home/paulf/scratch/valgrind/coregrind/m_libcbase.c:263:(.text+0xc8e): undefined 
> reference to `strlen'

So GCC figues out that the definition of VG_(strlen) is semantically equivalent 
to the 'strlen' built-in function.

<quote>
In addition, when a function is recognized as a built-in function, GCC may
use information about that function to warn about problems with calls to
that function, or to generate more efficient code,
</quote>

So it makes the VG_(strlen) body to be a call to strlen. That is fine as
long as you're linking to libc which we don't. But GCC does not know
that when compiling and by default targets a hosted environment.
Adding -ffreestanding won't help as it implies -fno-builtin.

I am wondering whether adding -nodefaultlibs to the compile flags helps.

<quote>
'-nodefaultlibs'
      Do not use the standard system libraries when linking.  ...
</quote>

If GCC still adds a dependency on a library function that I'm telling it won't 
be there - that would be quite odd...

Could you give that a try? Or may be change VG_(strlen) to return 
__builtin_strlen(str) ?

Thanks,
   Florian