OpenZFS Bug Ported to Zig
40 points by mtlynch
40 points by mtlynch
I’m a self-proclaimed Zig fan, but this seems a little disingenuous to me. @andrewrk uses const prolifically where the original code mutated variables in place.
If he had written
var cols = vdrz.vd_original_width;
// ...
cols = vdrz.get_logical_width(txg);
and
var psize = (asize >> ashift);
psize -= nparity * (divCeil(asize_shifted, cols) catch unreachable);
psize <<= ashift;
which is a more direct translation of the original code, then the Zig compiler wouldn’t have caught either mistake. Surely a fairer way to put it is that @andrewrk’s preferred style, and perhaps the style of idiomatic Zig caught the error, not the language itself.
If the original code had been written in the same way, using const everywhere, then clang/gcc would’ve caught the same error with -Wunused-variable.
I was just going to say… This blog post wouldn’t have even been made had it failed. Not a good signal.
I’m confused by this criticism.
If you develop a tool X that does everything tool Y does and has extra features, why wouldn’t you share results when you find a new thing that tool X does better? There’s no reason for anyone to publish thousands of articles about the ways that Zig and C have similar behavior.
There’s no reason for anyone to publish thousands of articles about the ways that Zig and C have similar behavior.
If C shows behaviour which is considered defective and zig is found to have the same defect, it absolutely makes sense to publish about it as an opportunity for reflection and finding ways to mitigate or fix the defect.
It’s definitely a lot more interesting than changing the problem in a way C compilers could also solve it then take a victory lap.
It’s definitely a lot more interesting than changing the problem in a way C compilers could also solve it then take a victory lap.
I get that criticism.
But @LenFalken seems to be saying that we should be suspicious of Zig because Andrew wouldn’t have published this blog post had he found the problem to be identical in Zig, which is a different critique that I don’t understand.
I always write Zig code to minimize unnecessary use of variables. Why would I make an exception for this snippet?
I don’t disagree and don’t think that’s a bad style. In fact most of my Zig code ends up looking the same. But the article opens with questioning whether Zig would have caught the bug. I’d argue that Zig didn’t catch it, but your adherence to that particular style of writing code did it.
I think the article is a solid argument in favor of maximizing the use of const, but doesn’t really make a case for Zig by itself.
Well, I hadn’t considered about unused assignments to variables until this lobste.rs thread, so I’m glad I made the post :-) Will be looking to add that to the language now.
Nice! I wonder if that could eventually be the precursor to some form of linear typing too. I’ve read that Hare is looking at exploring that space.
I think it’s pretty common for compilers to do the proper liveness analysis for unused variable warnings. Here’s an example where the liveness analysis has to compute a least fixed point to determine that x is dead:
void f(int n) {
int x = 0;
for (int i = 0; i < n; i++) x++;
}
And here’s clang -Wall:
<source>:2:9: warning: variable 'x' set but not used [-Wunused-but-set-variable]
2 | int x = 0;
| ^
If you refcount uses to calculate liveness (which is probably most people’s first idea) you end up computing the greatest fixed point instead, which is less precise when the dead variable’s dataflow graph has cycles. There was a fun GC paper a while back pointing out that RC and GC compute greatest and least fixed points respectively, and RC famously cannot collect cycles. The situation with static analysis of liveness is analogous. Paul Khuong has an old blog post with the observation that most people’s first instinct (“most people” meaning people who don’t work on compilers or static analysis) when solving a problem often leads to the suboptimal fixed point: https://pvk.ca/posts/62/
Does Zig have something like the clang static analyser that can catch the mistake as written in the original style? Arguably it’s a bug in gcc and clang that they need a separate tool for such a simple analysis: it should be a built-in warning like it is in Rust.
Spoiler: The bug would be easier to catch in Zig because the Zig compiler flags unused variables.
I’m surprised that OpenZFS happens without static analysis for unused variables, as I thought that was a pretty easy thing to check for. I wonder how many other bugs fall out if you run more static analysis on the ZFS codebase.
Except the bug would not have been caught in Zig without writing the code differently, because the variables were not unused.
And had the code been written differently, in such a way that Zig would have caught it, the C compilers could have caught it as well. The original post even mentions -Wunused.
Compilers don’t have to track unused variables, they can track unused assignments to the variables:
warning: value assigned to `psize` is never read
|
| psize <<= ashift;
| ^^^^^
|
= help: maybe it is overwritten before being read?
= note: `#[warn(unused_assignments)]` on by default
Yes I know that, I was in the original thread where this sort of things was being discussed. However that is not what the most widespread C compilers (GCC and Clang) do, nor is it what the Zig compiler does.