5 ms·
> Safe languages insert additional machine branches to do things like verify that array accesses are in-bounds. In correct code, those branches are never taken.
by bfkwlfkjf 1y ago
> Safe languages insert additional machine branches to do things like verify that array accesses are in-bounds. In correct code, those branches are never taken. That means that the machine code cannot be 100% branch tested, which is an important component of SQLite's quality strategy.
Huh it's not everyday that I hear a genuinely new argument. Thanks for sharing.
- beached_whale 1y agoI think those branches are often not there because it's provably never going out of bounds. There are ways to ensure the compiler knows the bounds cannot be broken.
- ChadNauseam 1y agoI wonder if this problem could be mitigated by not requiring coverage of branches that unconditionally lead to panics. or if there could be some kind of marking on those branches that indicate that they should never occur in correct code
- accelbred 1y agoYou'd want to statically prove that any panic is unreachable
- jonahx 1y agoSo is the argument that safe langs produce stuff like: // pseudocode if (i >= array_length) panic("index out of bounds") that are never actually run if the code is correct? But (if I understand correctly) these are checks implicitly added by the compiler. So the objection amounts to questioning the correctness of this auto-generated code, and is predicated upon mistrusting the correctness of the compiler? But presumably the Rust compiler itself would have thorough tests that these kinds of checks work? Someone please correct me if I'm misunderstanding the argument.
- oconnor663 1y ago> questioning the correctness of this auto-generated code I wouldn't put it that way. Usually when we say the compiler is "incorrect", we mean that it's generating code that breaks the observable behavior of some program. In that sense, adding extra checks that can't actually fail isn't a correctness issue; it's just an efficiency issue. I'd usually say the compiler is being "conservative" or "defensive". However, the "100% branch testing" strategy that we're talking about makes this more complicated, because this branch-that's-never-taken actually is observable, not to the program itself but to its test suite.
- lionkor 1y agoIt's not like that, the compiler explicitly doesn't do compile-time checks here and offloads those to the runtime. Rust does not stop you from writing code that accesses out of bounds, at all. It just makes sure that there's an if that checks.
- selcuka 1y agoOk, but you can still test all the branches in your source code and have 100% coverage. Those additional `if` branches are added by the compiler. You are responsible for testing the code you write, not the one that actually runs. Your compiler's test suite is responsible for the rest. By the same logic one could also claim that tail recursion optimisation, or loop unrolling are also dangerous because they change the way code works, and your tests don't cover the final output.
- binary132 1y agoIf they produce control flow _in the executable binary_ that is untested, then they could conceivably lead to broken states. I don’t believe most of those sorts of transformations cause alternative control flows to be added to the executable binary. I don’t think anyone would find the idea compelling that “you are only responsible for the code you write, not the code that actually runs” if the code that actually runs causes unexpected invalid behavior on millions of mobile devices.
- anitil 1y agoIt's the sort of argument that I wouldn't accept from most people and most projects, but from Dr Hipp isn't most people and Sqlite isn't most projects.
- deleted 1y ago[deleted]
- cogman10 1y agoIt's a bad argument. Certainly don't get me wrong, SQLite is one of the best and most thoroughly tested libraries out there. But this was an argument to have 4 arguments. That's because 2 of the arguments break down as "Those languages didn't exist when we first wrote SQLite and we aren't going to rewrite the whole library just because a new language came around." Any language, including C, will emit or not emit instructions that are "invisible" to the author. For example, whenever the C compiler decides it can autovectorize a section of a function it'll be introducing a complicated set of SIMD instructions and new invisible branch tests. That can also happen if the C compiler decides to unroll a loop for whatever reason. The entire point of compilers and their optimizations is to emit instructions which keep the semantic intent of higher level code. That includes excluding branches, adding new branches, or creating complex lookup tables if the compiler believes it'll make things faster. Dr Hipp is completely correct in rejecting Rust for SQLite. Sqlite is already written and extremely well tested. Switching over to a new language now would almost certainly introduce new bugs that don't currently exist as it'd inevitably need to be changed to remain "safe".
- manwe150 1y agoIf it was as completely tested as claimed, then switching to rust would be trivial. All you need to do is pass the test suite and all bugs would be gone. I can think of other reasons not to jump to rust (it is a lot of code, sqlite already works well, and test coverage is very good but also incomplete, and rust only solves a few correctness problems)—just not because of claiming sqlite is already tested enough to be bug free of the kinds of issues that rust might actually prevent.
- hypeatei 1y agoCouldn't a method like `get_unchecked()` be used to avoid the bounds check[0] if you know it's safe? 0: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.get_unchecked https://doc.rust-lang.org/std/vec/struct.Vec.html#method.get...
- oconnor663 1y agoYes. You have to write `unsafe { ... }` around it, so there's an ergonomic penalty plus a more nebulous "sense that you're doing something dangerous that might get some skeptical looks in code review" penalty, but the resulting assembly will be the same as indexing in C.
- hypeatei 1y agoI figured, but I guess I don't understand this argument then. SQLite as a project already spends a lot of time on quality so doing some `unsafe` blocks with a `// SAFETY:` comment doesn't seem unreasonable if they want to avoid the compiler inserting a panic branch for bounds checks.
- Ferret7446 1y agoIf you put unsafe around almost all of your code (array indexing) aren't you better off just writing C?
- aw1621107 1y agoPerhaps if the only thing you're doing is array indexing? Though I'm not sure that would apply in this particular case anyways.
- tomjakubowski 1y agoIn many cases LLVM can prove the bounds check is redundant or otherwise is unnecessary and will optimize it away.
- Aurornis 1y agoI guess I don’t find that argument very compelling. If you’re convinced the code branch can’t ever be taken, you also should be confident that it doesn’t need to be tested. This feels like chasing arbitrary 100% test coverage at the expense of safety. The code quality isn’t actually improved by omitting the checks even though it makes testing coverage go up.
- estebank 1y agoIn safety critical spaces you need to be able to trace any piece of a binary back to code back to requirements. If a piece of running code is implicit in code, it makes that traceability back to requirements harder. But I'd be surprised if things like bounds checks are really a problem for that kind of analysis.
- refulgentis 1y agoYeah sounds too clever by half, memory safe languages are less safe because they have bounds checks...maybe I could see it on a space shuttle? Well, only in the most CYA scenarios, I'd imagine.
- sgarland 1y agoBear in mind that SQLite is used in embedded systems, and I absolutely wouldn’t be surprised to learn it’s in space.
- manwe150 1y agoCritical applications like that used to use ADA to get much more sophisticated checking than just bounds. No certified engineer would (should) ever design a safety critical system without multiple “unreachable” fail safe mechanisms Next they’ll have to tell me about how they had to turn off inlining because it creates copies of code which adds some dead branches. Bounds checks are just normal inlined code. Any bounds checked language worth its salt has that coverage for all that stuff already.
- evil-olive 1y ago
- coolThingsFirst 1y agoThis is a dumb argument, it's like saying for a perfect human being there's no need for smart pointers, garbage collection or the borrow checker.
- ChrisRR 1y agoI can't figure out how you've come to that equivalence
- NobodyNada 1y agoIt's interesting to consider (and the whole page is very well-reasoned), but I don't think that the argument holds up to scrutiny. If such an automatic bounds-check fails, then the program would have exhibited undefined behavior without that branch -- and UB is strictly worse than an unreachable branch that does something well-specified like aborting. A simple array access in C: arr[i] = 123; ...can be thought of as being equivalent to: if (i >= array_length) UB(); else arr[i] = 123; where the "UB" function can do literally anything. From the perspective of exhaustively testing and formally verifying software, I'd rather have the safe-language equivalent: if (i >= array_length) panic(); else arr[i] = 123; ...because at least I can reason about what happens if the supposedly-unreachable condition occurs. Dr. Hipp mentions that "Recoding SQLite in Go is unlikely since Go hates assert()", implying that SQLite makes use of assert statements to guard against unreachable conditions. Surely his testing infrastructure must have some way of exempting unreachable assert branches -- so why can't bounds checks (that do nothing but assert undefined behavior does not occur) be treated in the same way?
- eesmith 1y agoThe 100% branch testing is on the compiled binary. To exempt unreachable assert branches, turn off assertions, compile, and test. A more complex C program can have index range checking at a different place than the simple array access. The compiler's flow analysis isn't always able to confirm that the index is guaranteed to be checked. If it therefore adds a cautionary (and unneeded) range check, then this code branch can never be exercised, making the code no longer 100% branch tested.
- dathinab 1y agothe problem is it's kinda an anti argument you basically say if deeply unexpected things happen you prefer you program doing widely arbitrary and as such potentially dangerous things over it having a clean abort or proper error. ... that doesn't seem right worse it's due to a lack of the used tooling and not a fundamental problem, not only can you test this branches (using fault injection) you also often (not always) can separate them from relevant branches when collecting the branch statistics so the while argument misses the point (which is tooling is lacking, not extra checks for array bounds and similar) lastly array bounds checking is probably the worst example they could have given as it - often can be disabled/omitted in optimized builds - is quite often optimized away - has often quite low perf. overhead - bound check branches are often very easy to identify, i.e. excluding them from a 100% branch testing statistic is viable - out of bounds read/write are some of the most common cases of memory unsafety leading to security vulnerability (including full RCE cases)
- sgbeal 1y ago> you prefer you program doing widely arbitrary and as such potentially dangerous things over it having a clean abort or proper error. SQLite isn't a program, it's a library used by many other programs. As such, aborting is not an option. It doesn't do "wildly arbitrary" things - it reports errors to the client application and takes it on faith that they will respond appropriately.
- deleted 1y ago[deleted]
- kazinator 1y ago> In incorrect code, the branches are taken, but code without the branches just behaves unpredictably. It's like seat belts. E.g. what if we drive four blocks and then the case occurs when the seatbelt is needed need the seatbelt? Okay, we have an explicit test for that. But we cannot test everything. We have not tested what happens if we drive four blocks, and then take a right turn, and hit something half a block later. Screw it, just remove the seatbelts and not have this insane untested space whereby we are never sure whether the seat belt will work properly and prevent injury!
- jkafjanvnfaf 1y agoIt's new because it makes no sense. There already is an implicit "branch" on every array access in C, it's called an access violation. Do they test for a segfault on every single array access in the code base? No? Then they don't really have 100% branch coverage, do they?
- prein 1y agoTake a look at their description of how SQLite is tested: https://www.sqlite.org/testing.html https://www.sqlite.org/testing.html I think a lot of projects that claim to have 100% coverage are overselling their testing, but SQLite is in another category of thoroughness entirely.