Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -404,16 +404,44 @@ read.
So `uname -r` is a *lying* oracle, which is worse than none: a consumer who
checks it is told a version, and the version is wrong.

Two changes, neither large:

- Derive the `release` field from the package version at build time, so that
`uname -r` answers.
- Have `OPENKAL_MUSL_TRACE` print one line naming the version before anything
else, so that a trace pasted into an issue carries its own provenance.
**Done in 0.9.0**, both halves:

- The `release` field is derived from the package version. `build.mcpp` reads
`mcpp.toml` and defines it, so the number is stated in one place; a manifest
it cannot read yields no definition and the field reports `unknown`, which is
a true statement rather than a false one.
- `OPENKAL_MUSL_TRACE=enosys` names the version **before the program runs**, and
whether or not anything is missing. That last part is the substance: a run in
which nothing was refused printed nothing at all, so "the version is right and
nothing is absent", "the variable did not take effect" and "this is not the
binary I think it is" were one reading.

The second matters more, because the trace is what a reporter is already asked
to paste. A report that carries its version is a report that cannot be answered
against the wrong one, which is the whole of what went wrong this round.
to paste. A report that carries its version cannot be answered against the wrong
one, which is the whole of what went wrong this round.

**measured**, and the third observation is the one that protects everybody who
is not debugging:

```
ok silent unless asked
ok the banner names 0.9.0
ok uname reports 0.9.0
```

and against the previous behaviour, with the constant put back:

```
banner: FAIL got=[]
uname: FAIL got=[0.5.0]
```

⚠️ The release field now MOVES AT EVERY RELEASE. Nothing here or in musl reads
it — `gethostname` and `getdomainname` are musl's only consumers of `uname` and
both read `nodename` — but a program above that compares it against a fixed
string will see it change. Recorded in `README.md`'s divergence table. The
trade was taken because the previous value was not merely uninformative: it was
wrong, and a consumer who checked it was misled rather than left uncertain.

---

Expand Down
63 changes: 63 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,69 @@ jobs:
MCPP_TARGET: ${{ matrix.target }}
run: bash tools/run-probe.sh examples/posix posix

# A BUILD STATES WHICH VERSION OF THIS LIBRARY IT HOLDS, AND THE STATEMENT
# IS COMPARED WITH THE MANIFEST RATHER THAN READ.
#
# This step exists because of what its absence cost. Two rounds of
# mcpplibs/openkal-linux#13 were answered against the wrong version: the
# only version-shaped thing a program could read was `uname`'s release
# field, which was the literal "0.5.0" through every release after 0.5.0,
# so a consumer who checked it was told a version and the version was
# wrong. The report that followed named versions its author believed in
# and could not verify, and neither could we.
#
# Three observations, and the third is the one that keeps this honest for
# everybody who is NOT debugging:
#
# 1. the banner names exactly what mcpp.toml says
# 2. `uname` reports the same string
# 3. WITHOUT the variable, the program writes nothing to the error
# stream at all
#
# Without (3) this feature would be free to become noise in every
# ordinary run, and nothing would say so.
- name: A build states its own version, and says nothing when not asked
env:
MCPP_TARGET: ${{ matrix.target }}
run: |
set -euo pipefail
want="$(sed -n 's/^version *= *"\(.*\)"/\1/p' mcpp.toml | head -1)"
[ -n "$want" ] || { echo "::error::no version in mcpp.toml"; exit 1; }
echo " the manifest says $want"

cd examples/posix
binary="$(bash "$GITHUB_WORKSPACE/tools/one-artifact.sh" posix)"

# (3) first, because it is the property an ordinary run depends on.
#
# `-eq' AND NOT `='. This step runs on every row, and BSD `wc' pads its
# count with spaces where GNU `wc' does not, so a string comparison
# reads " 0" and fails on a program that wrote nothing:
#
# ##[error]the program wrote 0 line(s) to the error stream
#
# The step above that does the same thing is `if: runner.os == Linux',
# which is why it never met this.
quiet="$("./$binary" 2>&1 >/dev/null | wc -l)"
[ "$quiet" -eq 0 ] || {
echo "::error::the program wrote $quiet line(s) to the error stream without being asked"
"./$binary" 2>&1 >/dev/null | sed 's/^/ /'; exit 1; }
echo " ok silent unless asked"

# (1) the banner. Each process announces itself once, so a program
# that starts another produces more than one line; they must agree.
banners="$(OPENKAL_MUSL_TRACE=enosys "./$binary" 2>&1 >/dev/null | grep '^openkal-musl [0-9]' | sort -u)"
[ "$banners" = "openkal-musl $want" ] || {
echo "::error::expected exactly 'openkal-musl $want'; got:"
printf '%s\n' "$banners" | sed 's/^/ /'; exit 1; }
echo " ok the banner names $want"

# (2) and the field a program reads without setting anything.
got="$("./$binary" 2>/dev/null | sed -n 's/^note: release=//p' | head -1)"
[ "$got" = "$want" ] || {
echo "::error::uname reports '$got' and the manifest says '$want'"; exit 1; }
echo " ok uname reports $want"

# ⭐ THE NAMES THIS LIBRARY MUST NOT TAKE FROM A PROGRAM ABOVE IT.
#
# Reported as openkal-musl#13. This one is a COMPILE-TIME criterion: the
Expand Down
22 changes: 20 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ the claim can be checked rather than repeated.

```toml
[dependencies]
openkal-musl = "0.8.0"
openkal-musl = "0.9.0"
```

It names no implementation and no platform: a C library is the one consumer that
Expand Down Expand Up @@ -118,6 +118,7 @@ answer that leaves a program wrong without telling it.
| an immediate answer about a started program | `waitpid(…, WNOHANG)` returns without the program having finished, but may wait up to one polling interval of the implementation beneath (one millisecond on Linux) | `kal_timeout_wait_process` takes a bound and openkal spells "no bound" as zero, so a caller that does not want to wait asks for the smallest bound there is. An environment rounds a bound up to what its clock can distinguish; a bound shorter than the clock is a promise no environment can keep. |
| closing a standard stream in a program being started | `posix_spawn_file_actions_addclose(&fa, 0…2)` makes the spawn report `ENOSYS`; above position two it is performed, because nothing there is inherited | openkal has no value meaning "no stream", and the value that looks like one — zero — means the opposite: the stream the caller has. Accepting the action and not performing it would hand a program the standard input its caller had just taken away. |
| starting a program upon a stream whose handle is zero | a caller that redirects its **output** onto its own standard input and then starts a program gets `ENOSYS` | `kal_spawn_streams` reserves zero for inheritance and `kal_stream` reserves nothing, so an implementation whose streams are the environment's own descriptors hands out zero for standard input. The two agree at position zero and cannot be told apart anywhere else. Reported upstream; refused here rather than answered wrongly. |
| ~~a version a program can read~~ | **answered since 0.9.0.** `uname`'s `release` field is this package's version, and `OPENKAL_MUSL_TRACE=enosys` names it on the error stream once per process before the program runs | It was the string literal `0.5.0` through every release after 0.5.0, so a program that asked was not left without an answer -- it was given a false one. ⚠️ It therefore MOVES AT EVERY RELEASE: nothing here or in musl reads it (`gethostname` and `getdomainname` are musl's only consumers of `uname` and both read `nodename`), but a program above it that compares the field against a fixed string will see it change. `sysname` is `openkal` and not `Linux`, so nothing can have been reading it as a kernel version. |
| **setting** the modification time of a directory | `utimensat` on a directory is refused, so `std::filesystem::last_write_time(dir, t)` throws. **Reading** it is unaffected and correct. The value differs by implementation and is the implementation's to give: `EISDIR` on Linux and macOS, `EACCES` on Windows — both measured | `kal_fs_set_modified` takes a `kal_file` and openkal has neither a `kal_dir` form of it nor a form that takes a name, so this port opens the name as a file, which a directory refuses. What a backend says about that is its own: one distinguishes a directory and one does not. Note that libc++ gives both overloads of `last_write_time` the same name in the message it throws, so the text does not say which of the two failed: the reading overload is `stat` and works on a directory. A caller using a lock directory's timestamp reads it to decide staleness and writes it to refresh the lock, and only the second fails. |

**⭐ What carries confinement here, since a mode word does not.** A program that
Expand Down Expand Up @@ -181,9 +182,22 @@ spent on exactly that question.
Each operation the dispatcher has no case for is then named on the standard
error stream, **once**, whatever the number of attempts:

openkal-musl 0.9.0
openkal-musl: no operation for system call 266

Three properties, each of them asserted in continuous integration because each
**The first line is the version, and it is printed whether or not anything is
missing.** That is the whole reason it exists. Before 0.9.0 a run in which
nothing was refused printed nothing at all, and three situations were then
indistinguishable: the version is right and no operation is absent, the variable
did not take effect, or this is not the binary the reader thinks it is. Two
rounds of [openkal-linux#13](https://github.com/mcpplibs/openkal-linux/issues/13)
were answered against the wrong version because of it.

A report pasted into an issue therefore carries its own provenance, and one
process contributes one such line — a program that starts another produces one
for each, and they must agree.

Four properties, each of them asserted in continuous integration because each
of the corresponding failures is quiet:

- **Nothing is reported unless the variable is set.** A diagnostic that appears
Expand All @@ -195,6 +209,10 @@ of the corresponding failures is quiet:
answer `ENOSYS` from cases of their own, each a decision with a reason
recorded beside it. Reporting those would name a facility as missing that this
port deliberately does not have, which is a different sentence.
- **The version named is the one in `mcpp.toml`.** It is read from the manifest
by `build.mcpp` rather than written out a second time, and the workflow
compares the printed line against the manifest. A version stated in two places
agrees until one of them is edited.

The report is written to the stream directly rather than through this library's
own output, because what failed may be the operation that output was about to
Expand Down
60 changes: 60 additions & 0 deletions build.mcpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,70 @@ std::string env_or_empty(const char* name) {
return v ? v : "";
}

// THE VERSION IS READ FROM THE MANIFEST RATHER THAN WRITTEN OUT A SECOND TIME.
//
// A program built on this library could not state which version of it it held.
// The only version-shaped thing it could read was `uname`'s release field,
// which was the string literal "0.5.0" and had never moved -- so a consumer who
// checked it was told a version, and the version was wrong. Two rounds of
// mcpplibs/openkal-linux#13 turned on a question the software gave no way to
// settle.
//
// A `defines` entry in the manifest would state the number a second time, four
// lines below where it is already stated, and the two would agree until one of
// them was edited. So it is read from the one place that has it. An unreadable
// manifest yields no definition at all rather than a wrong one, and the header
// that consumes this reports "unknown" -- which is a true statement, unlike
// the constant it replaces.
std::string package_version() {
const std::string dir = env_or_empty("MCPP_MANIFEST_DIR");
if (dir.empty()) return "";
std::ifstream in(dir + "/mcpp.toml");
if (!in) return "";

// The first `version = "..."` under [package]. Sections after it declare
// versions of other things -- dependencies name theirs the same way -- so
// reading past the first table would answer about one of those.
std::string line;
bool in_package = false;
while (std::getline(in, line)) {
const auto first = line.find_first_not_of(" \t");
if (first == std::string::npos || line[first] == '#') continue;
if (line[first] == '[') { in_package = line.compare(first, 9, "[package]") == 0; continue; }
if (!in_package) continue;
if (line.compare(first, 7, "version") != 0) continue;
const auto open = line.find('"');
if (open == std::string::npos) continue;
const auto close = line.find('"', open + 1);
if (close == std::string::npos) continue;
return line.substr(open + 1, close - open - 1);
}
return "";
}

int main() {
const std::string os = env_or_empty("MCPP_TARGET_OS");
const std::string fam = compiler_family();

// BEFORE THE EARLY RETURN BELOW, AND THAT IS THE WHOLE OF WHY IT IS HERE.
// The compiler family decides whether a runtime library is named; it does
// not decide whether this library knows its own version. Emitting this
// after that return would define the version under one compiler and leave
// it undefined under the other, and the sources would then disagree about
// what they report while compiling cleanly under both.
if (const std::string v = package_version(); !v.empty()) {
mcpp::rerun_if_changed((env_or_empty("MCPP_MANIFEST_DIR") + "/mcpp.toml").c_str());
// THE QUOTES HAVE TO SURVIVE A SHELL. The build file this reaches is
// executed by one, so `-DOKM_VERSION="0.9.0"` arrives at the compiler
// as `-DOKM_VERSION=0.9.0` and 0.9.0 is not an expression:
//
// <command-line>: error: too many decimal points in number
//
// naming a line in this file that is correct. Escaped here so that
// what the shell removes is the escape rather than the quote.
mcpp::cflag(("-DOKM_VERSION=\\\"" + v + "\\\"").c_str());
}

if (fam != "gcc") return 0;

// ⚠️ ELF NEEDS IT TOO, AND FOR THE SAME REASON THE PE BLOCK DID. The Linux
Expand Down
25 changes: 25 additions & 0 deletions examples/posix/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <pthread.h>
#include <time.h>
#include <sys/stat.h>
#include <sys/utsname.h>
#include <sys/wait.h>
#include <spawn.h>

Expand Down Expand Up @@ -387,6 +388,30 @@ int main(int argc, char **argv, char **envp) {
check(whole, "several pages are obtained in one allocation and every byte of it holds");
}

/* WHAT VERSION OF THE C LIBRARY THIS PROGRAM HOLDS.
*
* The release field was the string literal "0.5.0" through every release
* after 0.5.0, so a program that asked was given a false answer rather than
* no answer. It now comes from the manifest. This asserts that it is neither
* absent nor the placeholder a build that could not read the manifest would
* leave -- it deliberately does NOT assert a particular number, because the
* number moves at every release and an observation naming one would have to
* be edited by every release rather than checked by it.
*
* The build that DOES check the number is the workflow, which compares this
* line against mcpp.toml. Reported in openkal-linux#13, where two rounds were
* spent establishing which version a consumer had actually built. */
{
struct utsname un;
const int ok = uname(&un) == 0;
check(ok, "the system reports its identity");
if (ok) {
printf("note: release=%s\n", un.release);
check(un.release[0] != '\0' && strcmp(un.release, "unknown") != 0,
"the C library reports a version rather than a placeholder");
}
}

printf("-- failures: %d --\n", failures);
return failures ? 1 : 0;
}
2 changes: 1 addition & 1 deletion mcpp.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
namespace = "mcpplibs"
name = "openkal-musl"
version = "0.8.0"
version = "0.9.0"
description = "musl 1.2.5 redirected onto openkal: one C library, ported once, above every implementation of the specification rather than above one kernel."
license = "Apache-2.0"

Expand Down
8 changes: 8 additions & 0 deletions port/src/okm.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ int okm_fd_cloexec(int fd, int on);
int okm_fd_get_cloexec(int fd);
void okm_table_init(void);

/* Announces this library's version on the standard error stream, and only
* when OPENKAL_MUSL_TRACE asks for it. Called once from startup rather than
* from the first refused operation: a run in which nothing is refused would
* otherwise print nothing, and a consumer could not tell that from a run in
* which the variable never took effect. okm_syscall.c states the rest. */
void okm_trace_banner(void);
extern const char okm_version[];

/* The stream descriptor `fd' named when the program began, for fd in [0,3), and
* zero otherwise. What a spawn needs in order to tell a descriptor that has been
* redirected from one that has not; okm_fd.c records why the question is about
Expand Down
8 changes: 8 additions & 0 deletions port/src/okm_start.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,14 @@ void __okm_libc_init(void)
__init_tls(0);
__init_ssp(g_random);
okm_table_init();

/* AFTER THE ENVIRONMENT IS READABLE AND BEFORE THE PROGRAM RUNS. The
* banner asks openkal for a named value, so it cannot precede
* __okm_init_env; and it must precede main, because the question it
* answers -- which version is this -- is asked by a reader of the
* program's output, who cannot be sure the variable took effect if the
* only evidence is a report that may never be produced. */
okm_trace_banner();
}

int __libc_start_main(int (*main_fn)(int, char**, char**), int argc, char** argv,
Expand Down
Loading
Loading