Two mistakes I made when translating from the C header:
- The timeout and userspace_address2 pointers should be nullable.
- futex_wait() and futex_wake() are defined as static functions and
therefore not available as exported symbols. They're just thin
wrappers around futex() anyway so that's fine.
Having the matrix of test targets for incremental compilation in the
individual test manifests has turned out to be inconvenient for a few
reasons: the tests are almost certain to accidentally get out of sync,
disabling targets entirely is annoying to do, and incr-check needs to
take care to print the target in all error messages (which currently
does not always happen).
If I recall correctly, I originally designed it this way because it
allows targets to be disabled at the granularity of individual tests,
but there's an easier approach to that: just let a test manifest that it
should be *skipped* on a certain target! As skipping is the rare case,
and also the case you want readers to notice, it makes sense for *it* to
be explicitly specified, like how unit tests use `error.SkipZigTest`.
So, `incr-check` no longer runs through a list of targets specified in
the manifest. Instead, it accepts (and, in fact, requires) a single
target on the command line, and runs the test for that specific target.
If the file contains a `#skip_target` directive for that target, then
`incr-check` exits immediately, so we can still disable targets at
individual test granularity, but you can also disable a target for all
tests by just commenting it out of the matrix in `test/tests.zig`.
As a nice bonus, this also allows the build system to run different
incremental test targets in parallel, because the targets are now
different steps.
This definitely seems like a better way to split the work between the
build system and incr-check---sorry for getting this wrong initially!
...but not in the way you'd expect. We were actually tracking them in
cases where we shouldn't have been! We cannot track a declaration if its
parent namespace has been lost, because that will cause analysis
failures immediately, but if we excluded a type from the mapping due to
a major change (such as a struct turning into a union, or a field being
added), we were still including any trackable instructions inside the
container's field expressions (e.g. struct field type expressions). This
meant we were tracking a type declaration while losing tracking on its
parent namespace, with predictably disastrous results.
Oh, also, tracking for opaque types was just totally wrong (I think this
was a typo from a while back). We could map it to things other than
opaque declarations, and we never mapped declarations inside opaques.
So, uh, I fixed that too.
The intention behind the iterator was to avoid needing to allocate the
symbols, but in practice we need to allocate them anyway since we need
to reverse their order and don't have random access. The alternative
would be an N^2 algorithm.
In practice this isn't that bad, because even if the allocation fails,
we'll still end up printing the address, so the user still ends up with
the necessary information to reconstruct the crash. I don't think it's
worth it to try to set up some kind of ring buffer or return partial
results on failure, but may revisit this.
This fix reveals another bug--we need to display the inline site syms in
the reverse of the encoded order. The parent/child relationships are
actually encoded on the inline sites, but it's likely a bit fragile to
try to trace those, and also more complex. As long as there aren't
multiple matches this is fine, and if there are, tracing the parent/child
chain won't work anyway.
This will be relevant once #31605 is merged.
In general, stack traces do *not* contain unique addresses for inlined
frames, but for error return traces, they will after the above PR. This
bool indicates that code printing the trace should not try to resolve
inline frames since they're explicitly encoded into the instruction
addresses.
This is set as state on stack trace rather than passed into the
formatting methods as an argument, as it's not really a formatting
option--whether or not it's correct to resolve inlines is decided at the
time of capture!
Andrew Kelley