Skip to content

Conversation

@art049
Copy link
Member

@art049 art049 commented Nov 5, 2025

@not-matthias this change might create some inconsistencies in the inlining detection since the logic has been changed in upstream i think:

*inl_fnname = si->inltab[i].inlined.fn;

rh-mcermak and others added 30 commits August 9, 2025 02:40
Reuse the vmsplice syscall wrapper in coregrind/m_syswrap/syswrap-linux.c
for mips64 as well. And make sure arm64-linux and riscv64-linux also use
the POST vmsplice wrapper.
Specifically:

sys_getrlimit
sys_mincore
sys_tkill
sys_unshare
sys_splice
sys_tee
sys_vmsplice
sys_fanotify_init
sys_fanotify_mark
sys_kcmp
sys_bpf

https://bugs.kde.org/show_bug.cgi?id=508030
Do more fine-grained checks within sys_faccessat and sys_faccessat2
syscall wrappers.  Allow passing special value of VKI_AT_FDCWD as a
file descriptor.  Check for valid flags.

https://bugs.kde.org/show_bug.cgi?id=507853
Define __NR_statmount and __NR_listmount.
This update makes the source buildable on the mips32 arch.

https://bugs.kde.org/show_bug.cgi?id=508027
Command line flags that modify variable vex_control did not have the
desired effect when processed dynamically via VALGRIND_CLO_CHANGE.

The fix is to call the new function LibVEX_set_VexControl in
process_dynamic_option.

Fixes https://bugs.kde.org/show_bug.cgi?id=508093
No need to run random tests for binary IROps with 1-bit wide operands.
Defaults to "yes".
Add VexControl::iropt_fold_expr and observe it in fold_Expr_WRK.

This flag will be used shortly in the iropt-test program.
Users are not expected to use it which is why I'm not announcing
it in NEWS.
Multiple `make ltpchecks` failures seem tp be caused by
PRE(sys_fchownat) not handling VKI_AT_FDCWD properly.
This specifically impacts aarch64 test results these days.

https://bugs.kde.org/show_bug.cgi?id=508154
Using LHI is wrong as it sign-extends. Use IILF instead.
This has been wrong since day #1. Found by iropt-test.
When dlopen is used we might end up in an assembly powerpc/strcmp.S
variant that is optimized in a way memcheck cannot proof correct. We
try to intercept strcmp in ld.so, but might fail when strcmp is called
before our interception code is loaded. Having an hardwire for ld.so
strcmp (earlier intercept) would solve this.

https://bugs.kde.org/show_bug.cgi?id=508145
Multiplication is performed using 4-byte values.
There were two bugs:
(1) Iop_MullS8/16 operands weren't sign-extended
(2) No shifting and OR'ing required to access the result. r11[32:63] has it.

Found by iropt-test.
Easy as pie and fewer special cases for iropt-test.
Easy as pie and fewer special cases for iropt-test.
Bug was introduced by yours truly in e3f078d.
Use new function host_is_big_endian instead.
Was missing the memory header for std::make_unique,
and GCC doesn't like using smart pointers with cout so use
get().
That way we can check whether the result of constant folding matches
the value computed by the insn sequence that is generated to implement
a specific IROp. Bugs were found for all architectures. s390 has been
fixed. The other architectures are currently disabled.

- irops.tab modified to enable only those IROps that are actually
  implemented for a given architecture. Architectures considered here
  are amd64, x86, ppc32, ppc64be/le
- IRICB_iropt_payload amended to store both the folded and unfolded
  result
- In valgrind_execute_test call IR injection twice: with and without
  constant folding enabled (--vex-iropt-fold-expr=yes/no)
For
Bug 507720 - Review syscalls returning file descriptors (other platforms)

Some more recording of fd opens and closes
Lots of checks for the directory fd of *at() syscalls
Fixed a few bugs in pdfork and pdkill (and updated the testcase).
Updated a few message strings.
Started improving readlinkat - needs more work.
Florian Krohm and others added 13 commits October 21, 2025 17:45
gcc is still default, --cc=whatever overrides.
coregrind/valgrind works iff valgrind was installed beforehand.
But we don't know that. Use vg-in-place instead.
Make function check_valgrind_output actually check something..
Remove --check-prereq logic. It is not needed. I think I was doing some
experiment of using objdump to make sure the insns written out by
valgrind match those produced by objdump. And then there was different
behaviour of objdump in different versions.
Make sure that the configure check for openssl/crypto and libaio use
the primary arch flags. These are used in testcases for the primary
arch and should compile and link with the primary arch flags (which
are empty on most arches, but are set explicitly on e.g. mips).

Move the libaio check after the compiler check flags.
GNU gdb (Ubuntu 12.1-0ubuntu1) 12.1
with LANG=de_DE.UTF-8

Fixes these issues:
== 26 tests, 0 stderr failures, 0 stdout failures, 14 stderrB failures, 10 stdoutB failures, 0 post failures ==
gdbserver_tests/hginfo                   (stdoutB)
gdbserver_tests/hginfo                   (stderrB)
gdbserver_tests/hgtls                    (stdoutB)
gdbserver_tests/mcblocklistsearch        (stderrB)
gdbserver_tests/mcbreak                  (stdoutB)
gdbserver_tests/mcbreak                  (stderrB)
gdbserver_tests/mcclean_after_fork       (stdoutB)
gdbserver_tests/mcclean_after_fork       (stderrB)
gdbserver_tests/mcinfcallWSRU            (stderrB)
gdbserver_tests/mcleak                   (stdoutB)
gdbserver_tests/mcleak                   (stderrB)
gdbserver_tests/mcmain_pic               (stdoutB)
gdbserver_tests/mcmain_pic               (stderrB)
gdbserver_tests/mcvabits                 (stdoutB)
gdbserver_tests/mcvabits                 (stderrB)
gdbserver_tests/mcwatchpoints            (stdoutB)
gdbserver_tests/mssnapshot               (stdoutB)
gdbserver_tests/mssnapshot               (stderrB)
gdbserver_tests/nlgone_abrt              (stderrB)
gdbserver_tests/nlgone_return            (stderrB)
gdbserver_tests/nlpasssigalrm            (stdoutB)
gdbserver_tests/nlpasssigalrm            (stderrB)
gdbserver_tests/nlself_invalidate        (stderrB)
gdbserver_tests/nlsigvgdb                (stderrB)

All issues are similar to this:
  --- mcblocklistsearch.stderrB.exp
  +++ mcblocklistsearch.stderrB.out
  @@ -1,7 +1,8 @@
   vgdb-error value changed from 0 to 999999
  -Breakpoint 1 at 0x........: file leak-tree.c, line 42.
  -Breakpoint 2 at 0x........: file leak-tree.c, line 68.
  +Haltepunkt 1 at 0x........: file leak-tree.c, line 42.
  +Haltepunkt 2 at 0x........: file leak-tree.c, line 68.
   Continuing.
  +Warnung: Missing auto-load script at offset 0 in section .debug_gdb_scripts
   Breakpoint 1, f () at leak-tree.c:42
   42        t->l    = mk();   // B
   Continuing.
  @@ -63,4 +64,4 @@
     0x........[16] indirect loss record 4
     0x........[16] indirect loss record 5
   monitor command request to kill this process
  -Remote connection closed
  +Remote Verbindung wurde beendet
New qExecAndArgs packet has been added recently to GDB's remote
protocol.

The new qExecAndArgs packet is sent from GDB, and gdbserver replies
with a packet that includes the executable filename and the arguments
string that were used for starting the initial inferior.

On the GDB side this information can be used to update GDB's state,
the 'show remote exec-file' will reflect how gdbserver was started,
and 'show args' will reflect the arguments used for starting the
inferior.

When running valgrind together with GDB like this:

./vg-in-place --tool=memcheck --vgdb-error=0 /bin/ls -lh

~/build/gdb/gdb/gdb
target remote | coregrind/vgdb
We can now ask GDB to show the executable's arguments:

(gdb) show args

Argument list to give program being debugged when it is started is "-lh".

or the executable's name:

(gdb) show remote exec-file
The remote exec-file is "/bin/ls".
Fixed in f80358f server.c: handle qExecAndArgs remote protocol packet
After upgrading LTP testsuite version to 20250930 (tracked in bug
510169) munmap01 syscall test started failing.  It however turns out
that this testcase was substantially rewritten and the failure is
expected.  This update will silence mentioned false positive.

https://bugs.kde.org/show_bug.cgi?id=510292
fcntl F_GETFD is used to check if a file descriptor is valid, so only
make sure it isn't a valgrind fd, otherwise we might warn, with
--track-fds=bad for any bad fd.

https://bugs.kde.org/show_bug.cgi?id=510436
@art049
Copy link
Member Author

art049 commented Nov 5, 2025

but normally this should be covered by some tests

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 5, 2025

CodSpeed Performance Report

Merging #6 will improve performances by 12.07%

Comparing bump-3.26.0 (429e451) with master (ea96b20)

Summary

⚡ 1 improvement
✅ 51 untouched
🆕 4 new
⏩ 52 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 test_valgrind[valgrind-3.26.0, python3 testdata/test.py, full-no-inline] N/A 5.3 s N/A
🆕 test_valgrind[valgrind-3.26.0, python3 testdata/test.py, full-with-inline] N/A 7 s N/A
🆕 test_valgrind[valgrind-3.26.0, python3 testdata/test.py, inline] N/A 3.8 s N/A
🆕 test_valgrind[valgrind-3.26.0, python3 testdata/test.py, no-inline] N/A 3.5 s N/A
test_valgrind[valgrind-3.26.0, stress-ng --cpu 4 --cpu-ops 10, full-with-inline] 3.7 s 3.3 s +12.07%

Footnotes

  1. 52 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@not-matthias
Copy link
Member

Merged the changes from master into this PR. Had a small bug in the inline table lookup where we wouldn't return anything if the hashmap wasn't initialized.

@not-matthias not-matthias self-requested a review November 6, 2025 09:24
Copy link

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obviously did not read the whole patch, but let's merge this 💪

@not-matthias
Copy link
Member

not-matthias commented Nov 6, 2025

Looks like the inline tests are failing, but they still seem to work. This is the diff of the expected to the actual output:

--- inline-crossfile.post.exp	2025-10-21 17:35:17.908352039 +0200
+++ inline-crossfile.post.out	2025-11-06 11:36:30.159896899 +0100
@@ -5,3 +5,4 @@
 cfni=compute_product
 fe=inline-crossfile.c
 cfni=???
+=== Main Function Inline Markers ===

EDIT: Looks like the trace just includes an additional main function, without any sub-calls. Don't think it's worth it to modify the filter_inline script, I just added the line to the expected output.

fn=main
15 1
-1 1
fi=/home/not-matthias/Documents/work/wgit/valgrind-codspeed/callgrind/tests/inline-crossfile-helper1.h
cfni=compute_sum
-7 3
-1 3
+1 4
+1 1
-1 3
+1 1
-1 4
fi=/home/not-matthias/Documents/work/wgit/valgrind-codspeed/callgrind/tests/inline-crossfile-helper2.h
cfni=compute_product
+1 3
-1 5
+1 2
-1 4
+1 2
-1 4
fe=/home/not-matthias/Documents/work/wgit/valgrind-codspeed/callgrind/tests/inline-crossfile.c
cfni=???
+16 2
cfn=my_output
calls=1 -15 
* 982

ob=???
fl=???
fn=0x0000000000401228
0 4

fn=0x000000000483d29c
0 4

fn=0x000000000483d000
0 5

ob=/home/not-matthias/Documents/work/wgit/valgrind-codspeed/callgrind/tests/inline-crossfile
fl=/home/not-matthias/Documents/work/wgit/valgrind-codspeed/callgrind/tests/inline-crossfile.c
fn=main
26 3

ob=???
fl=???
fn=0x000000000483d000
0 2

fn=0x0000000000401000
0 7

fn=0x000000000483d030
0 1
cob=/nix/store/g8zyryr9cr6540xsyg4avqkwgxpnwj2a-glibc-2.40-66/lib/libc.so.6
cfi=???
cfn=__cxa_finalize
calls=1 0 
0 83

totals: 207852

@adriencaccia adriencaccia merged commit 429e451 into master Nov 6, 2025
17 of 18 checks passed
@adriencaccia adriencaccia deleted the bump-3.26.0 branch November 6, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.