0.6.1 --- the same bounded-transfer defect, and the tests that had never run - #15
Merged
Merged
Conversation
…tests that
would have caught it
`kal_timeout_read` and `kal_timeout_write` decoded their `kal_stream` argument
with `okm::unpack`, which is the decoder for an OWNED handle. A stream handle is
not one: these are this environment's own descriptors, and `kal_stream_read` and
`kal_stream_write` take them as such.
THE DECODE DID NOT FAIL, WHICH IS WHY THE CALL SITE READ AS CORRECT. `handle.h`
packs an owned handle as `(generation << 32) | (fd + 1)`, so a bare descriptor N
has exactly the shape of a packed handle naming N-1, and `unpack` accepts it
whenever the generation recorded for N-1 is still zero. The wait was therefore
performed upon descriptor N-1 while the transfer that followed was performed
upon N. Of the three standard streams only `kal_stdin`, whose handle is zero,
failed to decode; `kal_stdout` decoded to descriptor 0 and `kal_stderr` to 1.
This is openkal-linux's defect, in this file, unchanged. It was found there
first, through openkal-musl, and the two files carry the same four call sites
with the same two wrong. `kal_timeout_accept` and `kal_timeout_recv_from` keep
the decode and are correct: a listener and a datagram are owned. The four sites
divide exactly along the borrowed/owned line.
--- and the reason it was invisible here ------------------------------------
THIS PACKAGE'S OWN TESTS HAD NEVER RUN. `tests/` has held five suites since the
implementation was written and no workflow invoked them; what ran was the
specification's suite, which is a different instrument. The evidence that they
had never run is in the suites themselves: all five announced their success as
`openkal-linux: ...`, and `conformance_env_time.cpp` did not compile at all,
naming `kal::time::prop_wall_available`, which openkal 0.9 spells
`kal::time::wall_available`.
`mcpp test` is now a step, taken from openkal-linux unchanged, including its
assertion that every suite ran: a suite that discovered nothing reports success.
The five suites are corrected to name this implementation, and the stale
property name is fixed.
`tests/conformance_timeout.cpp` is new and is the observation that was missing.
There was no test of openkal.timeout in this package at all, and the
specification's suite examines it through observations that a wait upon the
wrong descriptor satisfies: a bounded read of the standard input is permitted to
expire, so an implementation that expires for the wrong reason is
indistinguishable there from one that is right.
Sixteen channels rather than one, because under the defect the wait was upon
descriptor N-1 and whether that expires depends on what occupies N-1. First in
`main`, because the mistaken decode corrected itself for every index at which an
owned handle had already been released.
--- measured -----------------------------------------------------------------
This system has no runner here, so the six suites were compiled and run against
openkal-linux, which provides the same interface: 6 passed, 0 failed. The new
suite was then run against openkal-linux with the defect put back:
0 of 16 bounded reads transferred
FAIL: a bounded read of a stream that has bytes waiting transfers them
Continuous integration on this branch is the criterion for this system.
`src/handle.h` records beside `unpack` that a word which was never packed is
accepted silently, and that a stream handle must not reach it.
… not retry
Continuous integration on the previous commit ran `tests/` here for the first
time. Five of the six suites held, including the new one; `conformance_process_task`
did not:
conformance_process_task ... FAIL (exit 1, 0.04s)
FAIL: the status it finished with is reported
The observation is correct and the test was wrong. It starts a program that
succeeds and reads its status, and it chose which program by spawning
`bin/true` and RETRYING with `usr/bin/true` if the spawn failed. That retry can
never run: `kal_process_spawn` reports whether the DUPLICATE was made, and the
image is replaced afterwards, inside a copy the caller no longer is. A path that
does not exist therefore produces `kal_ok` and a duplicate that finishes with
127. The first candidate was always taken and the second was unreachable.
This system has `/usr/bin/true` and no `/bin/true`, so the status observed was
127. openkal-linux carries the same test and the same dead retry, and holds only
because the first candidate exists there.
THE OBSERVATION AFTER IT HELD THROUGHOUT, UPON A PROGRAM THAT NEVER RAN. It read
`status != 0` for the program that fails, and 127 satisfies that. So of the two
observations, one failed for the right reason and the other passed for the wrong
one, from a single cause.
What changes:
* the candidate is chosen by asking `kal_fs_info` whether the name refers to
anything, which is an enquiry that can distinguish the two cases;
* `status != 0` becomes `status == 1`, the status `false` actually returns, so
a program that was never replaced no longer satisfies it;
* the failing observation prints the status it saw. `127' names an image that
was not replaced and `0' names one that ran, and without the number the two
arrive as the same line;
* the shell candidate is located the same way, for the same reason.
Measured against openkal-linux, which provides the same interface: the suite
holds. Continuous integration on this system is the criterion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is openkal-linux's defect, in this file, unchanged. mcpplibs/openkal-linux#19 is the counterpart and carries the full derivation.
The defect
kal_timeout_readandkal_timeout_writedecoded theirkal_streamargument withokm::unpack, which is the decoder for an owned handle. A stream handle is not one: these are this environment's own descriptors, andkal_stream_read/kal_stream_writetake them as such (fs.cpp:156,net.cpp:143,process.cpp:137-138,stream.cpp:6-8).The decode did not fail.
handle.hpacks an owned handle as(generation << 32) | (fd + 1), so a bare descriptorNhas exactly the shape of a packed handle namingN-1, andunpackaccepts it whenevergenerations[N-1]is still zero. The wait was performed upon descriptorN-1and the transfer uponN. Of the three standard streams onlykal_stdin, whose handle is zero, failed to decode;kal_stdoutdecoded to descriptor 0 andkal_stderrto 1.kal_timeout_acceptandkal_timeout_recv_fromkeep the decode and are correct: a listener and a datagram are owned. The four call sites divide exactly along the borrowed/owned line and the two that were wrong were the two holding a borrowed handle.Why it was invisible here, which is the larger half of this change
This package's own tests had never run.
tests/has held five suites since the implementation was written and no workflow invoked them. What ran was the specification's suite, which is a different instrument: it examines what every implementation must do, and these examine what this one does with the system beneath it.The evidence that they had never run is in the suites themselves:
openkal-linux: ...;conformance_env_time.cppdid not compile at all, namingkal::time::prop_wall_available, which openkal 0.9 spellskal::time::wall_available.So this change adds the
mcpp teststep, taken from openkal-linux unchanged including its assertion that every suite ran — a suite that discovered nothing reports success. The five suites are corrected to name this implementation and the stale property name is fixed.The criterion
tests/conformance_timeout.cppis new. There was no test of openkal.timeout in this package at all, and the specification's suite examines it through observations that a wait upon the wrong descriptor satisfies: a bounded read of the standard input is permitted to expire, so an implementation that expires for the wrong reason is indistinguishable there from one that is right.N-1and whether that expires depends on what occupiesN-1. For a channel made after another,N-1is the previous channel's writing end, upon which input is never reported.main, because the mistaken decode corrected itself for every index at which an owned handle had already been released.Measured
This system has no runner on the machine this was written on, so the six suites were compiled and run against openkal-linux, which provides the same interface:
and the new suite was then run against openkal-linux with the defect put back:
Continuous integration on this branch is the criterion for this system, and it is the first run in which
tests/has ever executed here.