Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix umockdev-vala testcase when Posix.open ("/dev/tty") succeeds #263

Closed
wants to merge 1 commit into from

Conversation

hdeller
Copy link
Contributor

@hdeller hdeller commented Dec 30, 2024

(this is a followup from pull request #261)
The umockdev-vala testcase still fails when called manually like this: ERROR:../tests/test-umockdev-vala.vala:284:t_usbfs_ioctl_static: assertion failed (ioctl (fd2, TIOCSBRK, 0) == 0): (-1 == 0) not ok /umockdev-testbed-vala/usbfs_ioctl_static - ERROR:../tests/test-umockdev-vala.vala:284:t_usbfs_ioctl_static: assertion failed (ioctl (fd2, TIOCSBRK, 0) == 0): (-1 == 0) Bail out!

I tested this on hppa, s390x and powerpc machines. Here is how to reproduce:

Run "meson compile"
Run "meson test"

All 7 testcases will succeed. Seems there is no problem, but please continue now:

You see the logfile mentioned like this:
Full log written to /home/deller/build/umockdev/build/meson-logs/testlog.txt

Open that "testlog.txt" file and search for the
"umockdev:fails-valgrind / umockdev-vala" test, e.g.:

test: umockdev:fails-valgrind / umockdev-vala
command: UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_CHECK_=3 TOP_SRCDIR=/home/deller/build/umockdev G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=131 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_TEST_ITERATION=1 PATH=/home/deller/build/umockdev/build:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games LD_LIBRARY_PATH=/home/deller/build/umockdev/build/:/home/deller/build/umockdev/build GI_TYPELIB_PATH=/home/deller/build/umockdev/build /home/deller/build/umockdev/src/umockdev-wrapper /home/deller/build/umockdev/build/test-umockdev-vala

Execute that "command" manually.
You will see that error on s390x and powerpc too, although the testcase succeeded above while running "meson test".

Sometimes, when called manually, the open("/dev/tty") succeeds, when called from "meson" it often fails and thus does not run the checks. In any case, the patch fixes the issue.

The umockdev-vala testcase still fails when called manually like this:
ERROR:../tests/test-umockdev-vala.vala:284:t_usbfs_ioctl_static: assertion failed (ioctl (fd2, TIOCSBRK, 0) == 0): (-1 == 0)
not ok /umockdev-testbed-vala/usbfs_ioctl_static - ERROR:../tests/test-umockdev-vala.vala:284:t_usbfs_ioctl_static: assertion failed (ioctl (fd2, TIOCSBRK, 0) == 0): (-1 == 0)
Bail out!

I tested this on hppa, s390x and powerpc machines.
Here is how to reproduce:

Run "meson compile"
Run "meson test"

All 7 testcases will succeed. Seems there is no problem, but please
continue now:

You see the logfile mentioned like this:
Full log written to /home/deller/build/umockdev/build/meson-logs/testlog.txt

Open that "testlog.txt" file and search for the
"umockdev:fails-valgrind / umockdev-vala" test, e.g.:

test:         umockdev:fails-valgrind / umockdev-vala
command:      UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_CHECK_=3 TOP_SRCDIR=/home/deller/build/umockdev G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=131 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_TEST_ITERATION=1 PATH=/home/deller/build/umockdev/build:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games LD_LIBRARY_PATH=/home/deller/build/umockdev/build/:/home/deller/build/umockdev/build GI_TYPELIB_PATH=/home/deller/build/umockdev/build /home/deller/build/umockdev/src/umockdev-wrapper /home/deller/build/umockdev/build/test-umockdev-vala

Execute that "command" manually.
You will see that error on s390x and powerpc too, although the
testcase succeeded above while running "meson test".

Sometimes, when called manually, the open("/dev/tty") succeeds, when
called from "meson" it often fails and thus does not run the checks.
In any case, the patch fixes the issue.

Signed-off-by: Helge Deller <[email protected]>
@martinpitt
Copy link
Owner

Sorry, you struck a hairy bug here. This PR breaks the test too much, we shouldn't assert the opposite of what the code is supposed to do. Nevertheless running TIOCSBRK on /dev/tty can't work, so let's rather find one that does. E.g.

python3 -c 'import os,fcntl,termios; fd=os.open("/dev/tty", os.O_RDONLY); print(fcntl.ioctl(fd, termios.FIONREAD, bytearray(8)))'

works in a real system, but not under libumockdev-preload. So that needs to be fixed. I'll work on that in the next days.

@hdeller
Copy link
Contributor Author

hdeller commented Jan 1, 2025

Thanks! Confirmed. Patch works so far for me.

@martinpitt
Copy link
Owner

@hdeller I suppose your previous "WFM" comment was for #262 😁

meson does strange things with /dev/tty, that's why the test behaves differently with meson test than with running the test binary directly. I locally changed it to run FIONREAD on /dev/stdout, which generally works better.

The main thing that goes wrong is the ioctl emulation. There is no explicit one in this test, so the _default handler kicks in which doesn't know about FIONREAD and then fails the ioctl with ENXIO -- presumably because the pipe fd is not valid in the "remote" process. That's the user-space ioctl emulation that @benzea wrote, and admittedly I don't understand it very well.

My gut feeling is that we shouldn't have that ioctl/_default handler. This should only be set up with an explicit handler, or at least only on emulated devices.

@martinpitt
Copy link
Owner

@hdeller I sent PR #264 to fix this properly. Thanks for spotting!

@martinpitt
Copy link
Owner

@hdeller Landed #264 and will do a release with that now. Cheers!

@martinpitt martinpitt closed this Jan 2, 2025
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.

2 participants