Bug 410757 - glibc wrapper discrepancy for preadv2/pwritev2 system calls across different versions
Summary: glibc wrapper discrepancy for preadv2/pwritev2 system calls across different ...
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: 3.15 SVN
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-09 09:47 UTC by Stefan Maksimovic
Modified: 2019-12-30 09:09 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
preadv2/pwritev2 check (1.67 KB, text/plain)
2019-08-16 13:18 UTC, Stefan Maksimovic
Details
preadv2/pwritev2 runtime prereq check (1.73 KB, text/plain)
2019-08-16 16:09 UTC, Stefan Maksimovic
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Maksimovic 2019-08-09 09:47:13 UTC
Running the memcheck/tests/linux/sys-preadv2_pwritev2 test on a system with glibc v2.27 and v2.28 we experienced
a failure which manifested itself in additional output on stderr.
Namely, in addition to the expected output for the preadv2/pwritev2 calls stderr contained output for preadv/pwritev calls as well.
The wrappers pertaining to v2.27 and v2.28 trigger a call to preadv/pwritev which produce the extra output.

Inspecting a bit further we found out that the above was due to the changes for the preadv2/pwritev2 system call wrappers inside glibc.
The versions which do not differ are 2.26 and 2.29.
Those on the other hand differ from the versions 2.27 and 2.28.
The difference can be easily observed here: https://elixir.bootlin.com/glibc/glibc-2.27/source/sysdeps/unix/sysv/linux/preadv2.c
(it concerns the if statement which checks the return value of SYSCALL_CANCEL).


I presume this should be handled using either a separate .exp or a suppression file.
Would the latter be the way to go?
Comment 1 Mark Wielaard 2019-08-09 10:09:58 UTC
So the issue is that sometimes glibc will try to emulate preadv2 with preadv if the kernel doesn't support preadv2?

Maybe the simplest solution to that might be to change the check for when to run the testcase?

Currently in configure.ac we test whether glibc provides preadv2:

AM_CONDITIONAL([HAVE_PREADV2_PWRITEV2],
               [test x$ac_cv_func_preadv2 = xyes && test x$ac_cv_func_pwritev2 = xyes])

Maybe we should change that test to one for __NR_pvreadv2 and __NR_pvwritev2 being defined (like HAVE_NR_MEMBARRIER) before that.

Then the test would only be build if the kernel actually has those system calls instead of just having glibc having fallback functions for them?
Comment 2 Stefan Maksimovic 2019-08-16 13:18:05 UTC
Created attachment 122172 [details]
preadv2/pwritev2 check

Thanks for the suggestion Mark,

Initially we tried using the approach you described but that did not suffice for the machine we encountered this problem on.
It seems that __NR_preadv2 and __NR_prwitev2 can be defined in unistd.h despite the kernel not supporting those system calls.
This suggests that the defines in unistd.h are maintained by the current glibc installation I presume?
Searching for __NR_preadv2 and __NR_prwitev2 in unistd.h on a machine with an older glibc version gave no results.

Instead of just checking for__NR_preadv2/__NR_prwitev2  presence like the membarrier test did, we opted to write a test
which would issue a syscall and subsequently check if the error code was set to ENOSYS.

This works fine in our case but it required to use an AC_RUN_IFELSE directive which will pose a problem in case of cross compilation as the test program will most likely not run at configure time.

With this drawback in mind, would this solution be acceptable?
If not, is there an another way to check for syscall support in the kernel without the need to actually run the test program at configure time?
Comment 3 Bart Van Assche 2019-08-16 14:24:37 UTC
How about adding a runtime prereq test? From drd/tests/annotate_ignore_rw.vgtest:

prereq: ./supported_libpthread

From memcheck/tests/amd64/sh-mem-vec256-plo-yes.vgtest:

prereq: ../../../tests/x86_amd64_features amd64-avx
Comment 4 Stefan Maksimovic 2019-08-16 16:09:20 UTC
Created attachment 122182 [details]
preadv2/pwritev2 runtime prereq check

Hello Bart,

Thank you for your input; yes, a runtime prerequisite would get rid of the cross compile issue mentioned in the comment above.
 
I've attached a patch which outlines the alternate approach.

Let me know what you think.
Comment 5 Bart Van Assche 2019-08-17 01:11:52 UTC
Please have a look at commit 0f7483d1d8a8 ("memcheck/tests/sys-preadv2_pwritev2: Check whether these syscalls are supported").
Comment 6 Mark Wielaard 2019-08-19 11:21:13 UTC
It needed this followup patch for older setups:

commit 4b39d3343762c53419cf16118c74e0004f900e32
Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date:   Sat Aug 17 18:27:22 2019 +0200

    Fix compilation problem when __NR_preadv2 __NR_pwritev2 are undefined
    
    check_preadv2_pwritev2.c: In function main:
    check_preadv2_pwritev2.c:12:12: error: __NR_preadv2 undeclared (first use in this function)
        syscall(__NR_preadv2, 0, NULL, 0, 0, 0);
                ^
    check_preadv2_pwritev2.c:12:12: note: each undeclared identifier is reported only once for each function it appears in
    check_preadv2_pwritev2.c:15:12: error: __NR_pwritev2 undeclared (first use in this function)
        syscall(__NR_pwritev2, 0, NULL, 0, 0, 0);

But I think it works OK now.
Comment 7 Julian Seward 2019-12-28 16:55:51 UTC
/me confused.  Do we need to do anything to fix this?
Comment 8 Stefan Maksimovic 2019-12-30 09:09:07 UTC
No need to do anything else for this issue, it was an omission on my part for not closing it after Mark's commit.

Marking it as resolved/fixed.