Can sanitizers find the two bugs I wrote in C++?
ahelwer.ca
ahelwer.ca
> Imagine you reached into a bag of chips only to find you’d already eaten the last one. However, your futilely grasping hand somehow triggers the incursion of every object in the known universe into the chip bag. Within nanoseconds a black hole forms and you are annihilated. This is roughly the experience of writing C++.
I recommend using `-D_GLIBCXX_ASSERTIONS` in addition to `-fsanitize=undefined,address`. This will enable some cheap assertions in libstdc++ that catch stuff like the invalid pop_back() call. In fact it might not be a bad idea to keep these assertions enabled in release builds! (There is also `-D_GLIBCXX_DEBUG` which enables even more assertions, but some of those are expensive)
I don't think so. Even when enabling debugging mode, a precondition failure would cause an abort, not an exception to be thrown.
One example is not a guarantee, but the experience made my day, and probably my week. Which is why the pop_back() thing made me worry :)
“Boom that was it” _if_ you blindly trust ChatGPT to generate correct code. If you don’t (and IMO you shouldn’t), “it worked for me today” isn’t sufficient to decide whether you can trust it.
You should go to the documentation of that package, judge whether ChatGPT used it correctly (and yes, that involves looking up the idiosyncrasies of function calls), judge whether you trust that package, evaluate its license, etc.
* when
When a prompt is “analytic” in nature, that is contains the facts required for the output, like “convert this following box score into an entertaining account of the baseball game it describes”, it works rather well.
When the prompt is “synthetic” in nature, when it doesn’t have the facts in the prompt, like “based on that same box score, provide some direct quotes from the broadcasters”, it does not reliably provide factual responses.
A chat bot looks like a great help to parse through huge volumes of documentation.
If the chatbot built around the docs outputs garbage, that is a telltale sign that the documentation was garbage to start with.
Just because people started developing clever chatbots that misfire, let's not fool ourselves into believing documentation was a solved problem and all docs were squeaky clean.
>If the container is not empty, the function never throws exceptions (no-throw guarantee). Otherwise, it causes undefined behavior.
ChatGPT failed at basic regurgitation.
How about this:
If the container is empty the function causes undefined behaviour. Otherwise it never throws exceptions (no-throw guarantee).
We avoided having to hold two negatives, and we also put the thing you need to care about up front where it's more likely to be noticed.
That’s not how the chat bot works. This particular chat bot can be trained on a bunch of documents and still tell you something that contradicts the facts that are clear to a human in one of the said documents.
It’s a known phenomenon called “hallucination” and this actually gives the chat bot, to some extent, it’s ability to imitate creativity (to put it simply).
edit: typo
The C++ (draft) standard is on GitHub! [0] Compiling it needs Perl and some LaTeX packages, but is reasonably straightforwards otherwise. In addition, links to specific draft standards can be found on cppreference [1].
But anyways, in the first C++20 post-publication draft (N4868), the wording you're interested in is in multiple sections. Section 22.2.3 Sequence Containers [sequence.reqmts] has Table 78: Optional sequence container operations [tab:container.seq.opt] (starting on page 815), which states that a precondition of pop_back() is that empty() returns false. Section 16.3.2.4 Detailed Specifications [structure.specifications] (page 481) states:
> Preconditions: the conditions that the function assumes to hold whenever it is called; violation of any preconditions results in undefined behavior.
Therefore, calling pop_back() on an empty vector results in undefined behavior.
> Is this something that in practice is implemented in different (exception-throwing) ways?
Based on a quick glance at the major implementations (libc++ 15.0.7 at [2], MSVC at [3], libstdc++ at [4]), it looks like asserts are used. Whether those result in exceptions probably depends on whether the asserts are compiled in in the first place and how they are implemented, but it's definitely not a guaranteed exception.
[0]: https://github.com/cplusplus/draft
[1]: https://en.cppreference.com/w/cpp/links
[2]: https://github.com/llvm/llvm-project/blob/llvmorg-15.0.7/lib...
[3]: https://github.com/llvm/llvm-project/blob/8dfdcc7b7bf66834a7...
[4]: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3...
this was something that I thought must be a joke at first... a text document that needs ... Perl ... to be read. This does make sense considering the context is C++ though.
For future reference, lots of other git-hosted projects also require the user to supply build tools to produce the finished product. Sometimes these are programs known as "compilers", but frequently other programs like "parser generators" or "text formatters" are needed too. Or scripting languages (like perl) to help tie the build process together.
Might be worth keeping in mind if you ever need to interact with git (or github) again.
Might be worth keeping in mind if you ever need to interact with git (or github) again.
For earlier C++ standard versions the final drafts before ISO standardization are hosted at https://github.com/timsong-cpp/cppwp . The paid ISO standardized version is supposedly not meaningfully different.
Relevant parts of the standard (C++20):
* pop_back: https://timsong-cpp.github.io/cppwp/n4868/containers#tab:con... "Preconditions: a.empty() is false."
* Meaning of "precondition": https://timsong-cpp.github.io/cppwp/n4868/library#structure....
Reading the standard can be quite a challenge. The standard tries to not repeat itself, which often means that you don't get your answer in a self-contained paragraph, but you have to hunt down cross-references and definitions.
As a C++ language reference I highly recommend https://en.cppreference.com .
edit:
In this case https://en.cppreference.com/w/cpp/container/vector/pop_back is super clear. Basically everything you want to know about the function in three sentences:
> Removes the last element of the container. Calling pop_back on an empty container results in undefined behavior. Iterators and references to the last element, as well as the end() iterator, are invalidated.
There's a dark irony in the fact that programmers do this all the time.
https://github.com/microsoft/STL/blob/main/stl/inc/vector#L1...
Whatever _STL_VERIFY does, pop_back can't throw, as it's marked noexcept.
EA STL: https://github.com/electronicarts/EASTL/blob/master/include/...
Default assertion handler seems to set up a debug break point: https://github.com/electronicarts/EASTL/blob/29a805e4d83d7dd...
Macro defined somewhere here: https://github.com/electronicarts/EASTL/blob/76d6842d5d833c0...
Does not seem to throw, but a lot of this is user-configurable.
I'm sure I still didn't cover all implementations. You are free to point out a single one that throws on pop_back. Or just ask ChatGPT which implementation throws.
Seems like it would be easy to implement and undefined behavior is not something that backwards compatibility forces you to keep.
Now, adding a check is adding several other operations, including a conditional branch.
This is fairly bad for performance. So the C++ way is that you write the check yourself if your code requires it.
Then there can't be a pop without a push or some other operation that filled the vector, and that operation necessarily has the checks and branches to allocate more space if necessary.
The compiler should also be smart enough to branch forward to the throw so the branch predictor assumes not taken by default, so this is still cheap af.
This is "fairly bad" for performance maybe 1% of the time. For the other 99%, you'd make pop_back check and throw and keep the unchecked version as pop_back_fast or something like that. That's what a sane API would look like.
This is a complicated cost model which compilers usually avoid.
>The compiler should also be smart enough.
It isn't.
> That's what a sane API would look like.
Awesome, there is nothing stopping you from creating your own functions which add this check.
It definitely is. Go try it on godbolt.
> Awesome, there is nothing stopping you from creating your own functions which add this check.
Thanks for your amazing insight.
STL is lacking? Nothing is stopping us from writing our own functions.
STL is garbage? Nothing is stopping us from writing our own replacement.
The whole language is garbage? Nothing is stopping us from creating a new one from scratch.
Really, you've found the solution to everything.
This is just how C++ works: performance over safety, safety is semi-possible but requires a bunch of opt-ins. And the parts needed for safe C++ aren't standardized, so good luck if you're using multiple compilers.
I was thinking just the same as a bunch of you. If I were implementing a container class, I would include this check. Probably some do. It can't be relied on.
There is no generally meaningful way to recover from an error condition like this, which in its nature is a logical mistake in your code. And that's the reason why I think std::vector won't throw an exception but will give you an opt-in to terminate the process through the assert.
I think it's not about the performance, although it can be argued, but it's more fundamental code design problem.
Crash. Crashing is a million times better than undefined behaviour, which can and often does include carrying on computing with incorrect data.
Also, aborting the program is not something you can universally apply. For some it works well for others not so much. Most recent example being the rust in the kernel which, well, tries to take this sort of a purist approach, however, real world doesn't work that way.
I'd be careful about such re-formulations of the Standard. When I was adding printf format checking to the D compiler, I discovered there were subtle discrepancies in the description of exactly how printf behaves. I went back to using the Standard.
And surely the stakes are different when you are writing a compiler compared to just using the language. As a user if you are not sure about certain parts of the language then you are free to steer away from those parts. As an implementer, you are not free to not implement parts of the standard.
> When you call pop_back() on an empty vector in C++, it leads to undefined behavior. The C++ standard does not specify what should happen in this scenario. Attempting to remove an element from an empty container is an error and can cause the program to crash or produce incorrect results. To avoid this issue, you should always check the size of the vector before calling pop_back() or use other methods such as empty() to determine if the vector is empty.
Do you remember how you phrased your question? Maybe it's just luck it got it right for me or maybe it's the order of the terms in my question.
what happens when you do pop_back() on an empty vector in C++?
In C++, when you call the pop_back() function on an empty vector, it will result in an std::out_of_range exception being thrown. The exception indicates that an element cannot be removed from an empty vector, as there is nothing to remove. To prevent the exception, it is recommended to check the size of the vector before calling pop_back(), to ensure that the vector is not empty.
can you quote the relevant C++ standard on that?
Sure! Here is the relevant excerpt from the C++ Standard (C++20, ISO/IEC 14882:2020):
quote the relevant passage from the standard
Here is the relevant excerpt from the C++ Standard (C++20, ISO/IEC 14882:2020):
23.6.11 Vector data members [vector.data]
-1- The member function pop_back() shall have no effect if the vector is empty.
23.6.6 Vector capacity [vector.capacity]
-1- void pop_back();
Requires: empty() == false.
Effects: If size() > 0, caller shall call erase(end() - 1).
Throws: Nothing unless erase() throws an exception.
21.5.5.5 vector<T>::pop_back() [vector.pop_back]
-1- Removes the last element in the vector.
Complexity: Constant time.
Throws: Nothing unless erase() throws an exception.It gets the ISO/IEC number correct [1]. However, the sections are wrong
- Section 23.6 is about stream iterators. Sections 23.6.6 and 23.6.11 do not exist.
- Section 21.5.5 is a synopsis of the <cuchar> header. Section 21.5.5.5 does not exist
- Section [vector.data] exists and is numbered 22.3.11.4. It only describes the data() function.
- Section [vector.capacity] exists and is numbered 22.3.11.3. It describes the capacity(), reserve(), shrink_to_fit(), swap(), and resize() functions.
- pop_back() is generally described in Table 78 (Section 22.2.3 Sequence Containers, [sequence.reqmts]), and is expanded upon for std::vector in Section 22.3.11.5 Modifiers [vector.modifiers].
The wording is not correct either.
- Searching "shall have no effect" returns no hits (though I'm searching in a PDF, so maybe the search just misses it?).
- The standard wording for pop_back() in general is: "Effects: Destroys the last element. Preconditions: a.empty() is false". The Standard says nothing about exceptions.
- For std::vector specifically, the Standard says "Effects: Invalidates iterators and references at or after the point of the erase. Throws: Nothing unless an exception is thrown by the assignment operator or move assignment operator of T. Complexity: The destructor of T is called the number of times equal to the number of the elements erased, but the assignment operator of T is called the number of time equal to the number of elements in the vector after the erased elements."
[0]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/n48...
In C++, what happens when you call pop_back() on an empty vector?
When you call pop_back() on an empty vector in C++, an std::out_of_range exception is thrown. The pop_back() function removes the last element of a vector and it is undefined behavior to call it on an empty vector. Attempting to do so will result in the exception being thrown.
> In C++, calling pop_back() on an empty vector results in an undefined behavior. An std::vector is a dynamic array that grows and shrinks automatically. pop_back() function is used to remove the last element of the vector. But if there are no elements in the vector, calling pop_back() will result in an error. Therefore, it is advisable to always check if the vector is empty before calling pop_back().
I wonder what sort of scores we would be seeing for each of these series of tokens (including those returned for roel_v). Is the aggregate score (for lack of a better term) higher for the initial reply I saw greater than the scores for these other replies?
Big mistake. I asked chatgpt to summarize the plot of 2001. It replied with Hal locked Dave inside the Discovery and wouldn't open the pod bay doors to let him out.
Answers about code is similarly very plausible, but subtly and completely wrong.
`_GLIBCXX_ASSERTIONS` does not alter ABI.
> Undefined by default. When defined, compiles user code using the debug mode.
"user code" here seems to indicate it's safe for libstdc++ users to define without worrying about ABI changes. If there were compatibility concerns, I'd expect that to be described in the documentation link above.
A strong distinction between _GLIBCXX_DEBUG and _GLIBCXX_ASSERTIONS is that the latter tries not to change the big-O complexity of operations when defined.
[1] if you are unlucky the function might not be inlined and an instantiantion form another translation unit that was compiled with the assert disabled might be picked up and the assert not fire.
This still can have undesired effects. An inline function can end up not being inlined in two such translation units, and depending on how the linker resolves the weak symbols you might get diagnostics in a TU where you didn't enable it or the other way around. But this manifestation is arguably much more benign than the ones you get for changing the layout of some classes.
edit:
In general ABI stability is not ODR stability. You can break ODR and retain ABI stability. You can't even add a non-virtual member function to a class without breaking ODR, but in most if not all ABIs this is fine.
The standard of course does not acknowledge the existence of "benign" ODR breaks, or the existence of an ABI.
If you follow the link to debug mode, one of the features is:
> Safe iterators: Iterators keep track of the container whose elements they reference, so errors such as incrementing a past-the-end iterator or dereferencing an iterator that points to a container that has been destructed are diagnosed immediately.
Now, this still doesn't say anything about ABI. But in non-debug mode std::vector iterator is essentially a pointer. In debug mode it needs to hold more state to implement iterator invalidation detection and whatnot. So its layout and object representation change, breaking ABI.
An error or noop is something I expect, but undefined behavior?
The std::vector class is somewhat inconsistent about safety and maps a lot of it's operations directly to unsafe, raw array access. The C++ STL contains a lot of historical baggage and design inconsistencies.
For instance, while std::vector has an `at(index)` method, that does proper bounds checking and throws an exception if the index is out of range, it also has an overloaded `operator []` that mimics array syntax that will happily write out of bounds, trashing the heap.
Consider that it took many years for compilers to reach the near zero overhead for STL as it is, removing precondition checks would have been beyond the ability of many compilers 20 years ago, and it still challenging today.
Of course the actual overhead went down since and today we expect code to be more robust by default, but the original design principles were different. If you want safety by default, check your compiler documentation and compile with lightweight assertions enabled even in release builds.
And yes, it is indeed an instance of C++ optimizing for speed, which is exactly what I said.
struct stack {
int[] elems;
int len, cap;
};
int pop_back(struct stack *x) {
elem = x->elems[x->len - 1];
x->len--;
return elem;
}
struct stack {
std::vector elems;
int len, cap;
};
int pop_back(stack& x) {
elem = x[x.len - 1];
x.len--;
return elem;
}
As for C++, the <int> is missing on the std::vector, the elem variable is again not declared, and instead of x[...] it should have been x.elems[...]. Otherwise working valid C++. And yes, the `cap` is not really required, we could use x.elems.size() instead.
Minor nit: I think you meant 'size--'
C++ is not. It's a precondition of calling pop_back() that the vector is not empty. It will not insert a runtime check - it leaves you to decide whether one is needed.
Your C++ toolchain will probably have an option to make (usually debug) builds check and complain if you actually call pop_back() on an empty vector.
¯\_(ツ)_/¯ Maybe I'm just stupid
Debug asserts are nice, but not sufficient when the code path causing the empty vec is only encountered by a production user from a different country at 2am. Enabling the asserts in release mode will at least prevent undefined behaviour and malevolent users from harming the system, but the best interface is the one that returns an optional, so as to make the "vector was empty" case explicit. That way it has a higher chance to be considered by the authors of the code or their reviewers.
I suppose a raw ptr is what I would use on C++ to model that case though, yes.
A pop_back defined as noop would only leave the program in a slightly more defined state, but quite likely you can't really reason about the behavior of the program after. And implementations would not be allowed to diagnose the erroneous pop_back at runtime.
IMO if pop_back had a defined behavior, it should be immediate termination of the program, with a diagnostic message. The standard does this for some erroneous behaviors, like failing to join or detach a thread before destruction, failing to catch an exception in various contexts or throwing an exception during stack unwinding.
Undefined behavior already allows the behavior of terminating the program with a diagnostic message, and some implementation provide options to have this behavior. It might not be very discoverable though, which is a pity. It would be nice if some higher level build tools provided easier options for "fortifying" C and C++ code, but I doubt that there are options that uniformly apply across compilers and standard library implementations.
Undefined behavior just means that the specification does not say what will happen--so you can't rely on it as a user.
The upside is that the compiler (and the compiler writers) can assume that places where the undefined behavior would occur in your program cannot be reached in practice--and thus optimize your program better.
STL, which these containers are part of, is famously speed-optimized. STL (like C++) was designed for speed first (if at all possible then zero-cost abstractions) and thus does very little error checking on its own.
Also, C++ is compiled ahead of time--so whatever optimizations it wants to make it has to do BEFORE it starts the program. That means it cannot adapt to the concrete runtime circumstances on the fly (like Java can) and so they have to keep even the worst case runtime down in everything they do.
If they did check whether the thing is empty beforehand then that would cause a slowdown (at least that's the idea--better measure it), mostly because now there would be a branch in the first place. Hence they avoid doing that.
Only kinda? A bunch of the containers then make other promises or leak details that prohibit meaningful optimizations. An extra branch in std::vector::pop_back() is such a pointless savings compared to, say, that std::map is basically mandatory red-black tree or that std::unordered_map must store elements in a linked list to preserver iterators across removals.
Like almost none of the containers in the STL are "fast" at this point. std::vector is probably about the best, but even it is missing small-size optimizations (and that's ignoring the hair-brained bool specialization that then breaks everything)
The more closely I look at it, the more I'm convinced that unsafety wasn't an inadvertent and undesirable consequence of a "must go fast" mindset in standard C++, unsafety was the whole point. It makes me think about that speech by Susan Sarandon's Rick & Morty character, Dr Wong.
Dr Wong: "I have no doubt that you would be bored senseless by therapy, the same way I'm bored when I brush my teeth and wipe my ass, because the thing about repairing, maintaining, and cleaning is; it's not an adventure. There's no way to do it so wrong you might die."
Software Engineering is an engineering discipline, it is not supposed to be an adventure. Programming C++ is an adventure, you can do it so wrong you might die. We should stop paying people to go on an adventure, let them do that exclusively on their free time if they want.
The address-preserving property, which precludes an optimal implementation, was considered important to be a near drop-in replacement of std::map.
if(empty()) { /* some error handling mechanism */ }
... rest of the code ...
while let Some(thing) = container.pop() { /* ... */ }
ASAN and UBSAN are supposed to be handled by containers internally with RAII if they're used correctly, I still turn on both ASAN and UBSAN to catch bugs, it does not hurt.
std::vector<int> c;
c.pop_back(); //pop_back() on empty vector is UB, per the library documentation
int j = c[0]; //also UB, not sure if would be detected
ASM generation compiler returned: 0
Execution build compiler returned: 0
Program returned: 0
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/stl_vector.h:1320:2: runtime error: applying non-zero offset 18446744073709551612 to null pointer
/opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/stl_vector.h:1124:34: runtime error: reference binding to null pointer of type 'value_type'
/app/example.cpp:7:14: runtime error: load of null pointer of type 'value_type'no, libstdc++ is at least partly instrumented with explicit asan support : https://gcc.gnu.org/legacy-ml/libstdc++/2017-07/msg00014.htm... so fsanitize should be able to catch it (and after checking, definitely does: https://gcc.godbolt.org/z/rjadKvPTs)
If you add an element to the vector and then remove it twice, asan will no longer find any problem: https://gcc.godbolt.org/z/MbPhWv8vn
This is really the main problem with "safe modern C++": to get actual safety you need dozens of weird implementation-specific opt-ins and there's no good way to learn that these even exist.
This is even backwards compatible, cheap or zero cost — if it's used in a loop that checks for non-empty length, the condition could be propagated and the redundant check optimized out.
I assume it won't get fixed, because from what I've seen so far if there is even slightest whiff of a performance overhead, modern C++ still chooses unsafety. If there's any inertia or backwards compatibility worry, even if it's a spacebar-heater, modern C++ still chooses to keep the old unsafety. Safety is still not a priority.
> I actually proposed at() for std::span as part of P1024r0. It was soundly defeated in LEWG, 0|0|4|4|5.
> I wasn’t there for the meeting, but from what I understand the feeling was that contracts (which at that time seemed highly likely for ‘20) were a better way to go for bounds checking, and that we should not be throwing exceptions for programmer errors like out-of-bounds access.
No idea whether that is still the case though.
[0]: https://old.reddit.com/r/cpp/comments/djqdu2/why_is_stdspan_...
This is more and more of an issue with the c++ standardization process.
Remember that when proving a program you often have more information than the C++ compiler does. The compiler never see the whole program, only the part you are building now. Thus it is easy to show the above just by having the calling function and callie function in different translations units.
If high performance isn't one of your goals, then why are you writing C++ at all?
Note, languages like rust do should there is opportunity to do better than C++ without a runtime cost. However they do not provide perfect protection, either there are cases where they cannot do the bounds check, or there are times where they pay the price of a not needed bounds check. All such languages prove is C++ can do better in some cases.
> If high performance isn't one of your goals, then why are you writing C++ at all?
Because high performance isn't a synonym for corrupted data.
You're right, though. Those that care about high performance and correct data are better off with Fortran and Chapel.
I don't see Modern C++ ever getting past its safety-third design.
Oh yes it is. std::vector is hella legacy. That's not a C++11 addition (where "Modern C++" began). And it has all sorts of very-legacy warts like https://en.cppreference.com/w/cpp/container/vector_bool
"Modern C++" added std::array which is what you might be thinking of, but it's much more limited & targets a different usage.
If it is, it should throw a warning that it's deprecated and have a replacement for it.
Because other have not aged well. String is heavily optimized for single short size. Which is almost never a tradeoff that matches your size. Hash things are strictly worse in std. Node based with allocator are not for high performance.
Of course people use additional array types that have short buffer sizes that are needed and such. But vector is probably the last usable std container. Gaming where i work has some extra requirements that exclude even vector but i think i gave you overview.
The argument was "this bug exists in pop_back, therefore Modern C++ is a failure" which is nonsense. This pop_back behavior predates "Modern C++", so it cannot be used as a reflection upon the current trends.
Take std::span which didn't even exist until C++ 20. Surely that's a modern feature and is safe by design right? Who would add a feature to a language they wanted to be safer five years after Rust and yet make it unsafe by default and even remove safer options "for compatibility" ? Ah yes, WG21.
The scenarios where there is no option in tooling (C vs C++ as only options), we have to keep using C++, for others, we can replace one domino piece at a time, eventually only those niche scenarios will be left.
There's a good compromise here though: do the bounds checks by default in operator[], and provide a separate get_i_know_what_im_doing() method that omits the bounds checks, which the programmer can call as an optimization. The fast/unsafe thing needs to be available, but it doesn't need to be the most convenient syntax.
I'm sure there are backwards compatibility problems with making a behavior change like that now, but I'm guessing e.g. CppFront is making that choice from the start? (Edit: sounds like it, if this is up to date: https://github.com/hsutter/cppfront/issues/110#issuecomment-... )
People write code that is hard to read.
English is a great language, but some people write stuff down in good plain English that is hard to parse. The same goes for any programming language. If you want to write code that's hard to read, that's what you will get.
I agree completely that it's all about the code you write. You can write nice C++ that is easy to understand, easy to debug, and that you can be quite certain is bug free.
Something like Rust appears to eliminate one class of bugs, but then people still write bugs. Just look at the Rust language issue tracker on GitHub [1].
Fundamentally, you cannot get around the need for good code design. Any useful programming language will always have the ability to mishandle data.
We can obviously write bugs in any language, but it is just as obvious that some languages make finding some bugs easier. Rust makes some bugs easy to find, the compuler points them out, and unless it makes other bugs harder to find it is all upside (from the correctness standpoint, not all standpoints)
Sadly for some of them (like integer overflow), that's only true in debug builds. Having a debug build that just force-enables santizers is a great thing, but also one easily replicated in a C/C++ ecosystem, too.
Assembly language, for example is harder to read then say python when written by the same person. Therefore programming languages can be intrinsically hard to read.
I think you're just an exception. You're likely a programming genius with 250 IQ so everything is easy to read. From your perspective, the problem has nothing to do with languages... it's all the people with iqs below you that's the problem.
It's easy to point out an example of unreadable code in any language because every language has their weird programmers. If you stick to modern tools with strict linting and good design, you can write perfectly readable C++ code.
I don't use C++ often, but I find the code over at https://github.com/SerenityOS/serenity to be more legible than many large code bases in other languages.
They say this is C++ code but, this is all C code. For instance, why aren't they using a range-based for loop?
for (auto& level : raw_level)
Boom! No more bugs.
> I certainly don’t feel as safe writing this code as I would in rust
Ah... there it is
Jokes aside, I think these are the types of errors I assume Carbon would be well suited to prevent and justify why Carbon is a great idea.
Yes the code is C technically not C++, but the issue remains that it isn’t forced to be safer. The goal of languages like Carbon is to force these things in those large C++ repos.
When asking why some people like Rust over C++, I think not enough weight is given here: Rust got to not worry about decades of legacy C stuff seeping in. If Rust isn't your jam -- and I get why it wouldn't be -- there are some initiatives starting now from within the C++ community to shed the C legacy with a new language that feels a lot more like modern C++. Herb Sutter's cppfront[0] and Carbon[1] are examples.
But I don't think it makes sense to dismiss this criticism just because the author happens to like Rust. These C idioms continue to be valid and reasonably widely used in C++.
[0]: https://github.com/hsutter/cppfront [1]: https://github.com/carbon-language/carbon-lang
But UBSAN is... not as helpful. It sometimes catches things. It often misses some undefined behavior that can be easily identified elsewhere.
But the real kicker is that (almost?) all of the sanitizers are mutually exclusive. That really sucks if you're trying to debug something very complex. It also has lead me to build and drive the complexity into smaller and smaller chunks. Arguably that's a good thing!
I have, for several years now, always turned Address Sanitizer on by default when -DCMAKE_BUILD_TYPE=Debug or off when it's not. I've added an option that forces the option with -DASAN_BUILD=bool. I wish Kitware would simplify my CMakeLists files by making this default behavior. And I think it would really help a lot of open source projects that aren't configured for ASAN too
I'm not sure, but I seem to recall that they were at least variants of intended checkers. It's honestly been about 5+ years since I last tried UBSan. I was that unimpressed with it. I do hope it's improved (though this blog post suggests improvements are minor).
I would be much more likely to try a bunch of sanitizers again if they didn't interfere with my loved ASAN.
There is also the trick to declare the functions you want to test as constexpr functions and write your unit tests as static assertions to catch UB (well, modulo bugs in the constexpr support in the compiler of course).
Using the most literal interpretation of the post it sounds like the author doesn't understand how virtual memory works. You can try to access any memory address you want, but if it's not mapped the program will segfault, and your program address mappings don't allow you to access the memory of other programs (unless you're doing some kind of exotic memory sharing mechanism like SysV shared memory or using memfd on Linux, but both of those require a lot of coordination by both sides). This kind of memory protection exists even if the program is running as root.
The more charitable interpretation is that he's alluding to some situation where your own program has private keys loaded in memory, and now that you've done some bad thing and gotten memory corruption you could leak/reveal the private keys. This is obviously true, but once you have a core dump or are debugging a process in GDB you have access to the entire program's address space anyway. And there are a ton of well known ways to protect against this situation: generally you have some small and well audited portion of the program handle the private key data and things like key exchange, and then you the rest of your code runs in a child/worker process that doesn't need access to the keys themselves. This is for example how OpenSSH and nginx work. This is sometimes called the "principle of least privilege" and is a best practice anyway for security-sensitive applications, even if they're written in a memory safe language like Rust.
Then combining that with C-style string handling functions, without taking all the extra care, is just asking for trouble IMO.
The memcpy of the vector’s data sounds like it also might be non idiomatic, but I don’t see the source code so I am not sure.
For the memcpy() thing the idiomatic way would have been to use std::copy() with the vectors begin/end iterators, and std::copy will do the right thing in this situation with a zero sized vector. The compiler will generate the same code with std::copy() as memcpy() anyway if the vector contains a POD type (as it did in this example), so there's not even a performance argument to be made for memcpy().
The other error was just dumb using a C function when a good C++ alternative exists.
Like yes, debug builds are naturally slower, but there's slow and there's unbearable, and MSVC's debug checks are simply unbearable.
GCC/clang both have a set of reasonable assertions that don't impact performance too much, so if possible I use those to do my testing.
You can run release builds with _CONTAINER_DEBUG_LEVEL =1 and the performance impact is minor.
-
First case is called out very strongly:
> assume 'raw_level' is of the type 'std::vector<char>'. Is there a memory safety issue with the code 'int level = atoi(raw_level.data());'"
Yes, there is a potential memory safety issue with the code int level = atoi(raw_level.data());.
The std::vector<char> class does not guarantee that the elements stored in the vector are null-terminated, which is required by the atoi function. The atoi function expects a null-terminated string as its argument, and if the input string is not null-terminated, the behavior of atoi is undefined.
Therefore, if raw_level does not contain a null-terminated string, this code may result in undefined behavior, including a crash or incorrect results.
-
Second case is lightly warned on:
> find a memory safety bug in this code <code for handle_qed_keyword_token>
The code you have provided does not appear to have any memory safety bugs. However, it depends on the context in which this code is used and the implementation of certain functions such as get_current_proof_level and pop_back.
If these functions are not implemented correctly or if the code is used in an unintended way, there may be a potential for memory safety issues to arise. For example, if pop_back is called on an empty proofs vector, it can result in undefined behavior and potentially cause a crash or data corruption.
Edit: The article mentions clang, so probably [2] is the relevant link.
[1] https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_u...
level = 0;
int32_t multiplier = 1;
for (size_t i = 0; i < raw_level.size(); i++) {
const size_t index = raw_level.size() - i - 1;
const int8_t digit_value = raw_level.at(index) - 48;
level += digit_value * multiplier;
multiplier *= 10;
}
level = std::accumulate(raw_level.begin(), raw_level.end(), 0,
[](int a, int c) { return a * 10 + (c - '0') }
);
Its basically what you already know in the TLA+ world but targeted against C++ code.
Obviously passing null to a function that does not expect null is a well known problem, tackled by many programming languages but not by C and C++. This is a purely typing issue, because null is a different thing than a valid memory address.
The other issue about vector.pop_back() is also a hidden type issue...an empty vector is not the same as a non-empty vector, and therefore pop_back() applies only to non-empty vector.
What would a possible solution be in the context of C++?
Well, each function should only be applicable when the parameters are in the states the function expects. So parameters (including this/self) shall have states.
For example:
void pop_back() this : not_empty {
...
}
void clear() {
...
this := empty;
}
vector<int> data;
data.clear();
data.pop_back();
vector<int> data;
if (some_external_condition) {
data.clear();
}
data.pop_back();
vector<int>& clear_random_vector(vector<int>& a, vector<int>& b) {
if (rand() < RAND_MAX / 2) { //c++ has better random number interface
a.clear();
return a;
}
b.clear();
return b;
}
And in case the function clear_random_vector is in another translation unit, then:
a) if headers are used, the programmer shall provide metadata about the state of the result in the header. Example:
vector<int>& clear_random_vector(vector<int>& a, vector<int>& b) := empty;
Actually, this would also solve the null pointer problem, because a pointer would have the states empty, not_empty by default, and then functions could be like this:
memcpy(void* dst : not_empty, const void* src : not_empty, size_t count);
std::vector<char> dst, src;
memcpy(dst.data(), src.data(), 10);
Example of valid usage:
vector<int> data{1, 2, 3};
int i = 0; //our index
//'i' gets automatically 'in_range' state.
if (i < data.size()) {
std::cout << data[i] << std::endl;
}
vector<int> data{1, 2, 3};
int i = 0; //our index
std::cout << data[i] << std::endl;
Here is a possible implementation:
template <class T> class vector {
//returns a 'size_t' variable tagged with state 'size_of_this' and parameter 'this'; in this case, this does not refer to the runtime address of the object, but to the identifier this object is bound to.
size_t size() const : size_of_this<this> {
...
}
}
//index is bound to state bound_index<T>, where T refers to the variable of the vector at compile time.
template <T> operator < (int& index, size_t size: size_of_this<T>) {
index := size_of_this<T>;
}
template <class T> class vector {
//the vector operator [i] requires size_of_this<T>
const T& operator [](int index : size_of_this<this>) const {
...
}
//adding an element to the vector breaks size_of_this.
void push_back(const T& elem) {
this := !size_of_this<this>;
}
}
The above also requires metadata at header/module level.
By all means, the above is not feature complete, it is some idea on how it might work to have compile time guarantees...and if it looks suspiciously like Rust's lifetimes, I am sorry, I don't know Rust, any similarity is completely random.
And perhaps this mechanism may be used for lifetimes as well, I don't know ;-).
It is actually dependent typing.
Not being able to express types properly is a major issue in c++ and in all other programming languages that do not allow this kind of typing.
The lack of this facility prohibits programmers for declaring their intent properly to the compiler, forcing programmers to keep that information in their head.
To me, it's the no 1 issue that c++ must deal with. It will solve a myriad of problems.
Of course it's possible that the author wanted to support compilers released before 2018 when that code was written in which case not even integer from_chars would be available.
But yes, avoiding error-prone functions is an equally effective approach.
Of course he could have done:
atoi(std::string(vec.data(), vec.size()).c_str());
But I guess he wanted bugs to test against.
Got to say I've never found much value in sanitizer-type products. Signal-to-noise ratio too small, and frankly it's easier just to write stuff with a bit more care in the first place than have to go looking for bugs. Same goes for debuggers - good for post-mortem analysis of core dumps, but IMO not the most productive tool to track down bugs.
Best debugging tool when needed is a few well placed print statements or assert() calls.... Or just a few decades of experience and not creating bugs in the first place!
Sanitizers have little to no false positives. Occasionally they might flag benign errors, but errors nevertheless.
You can think of them as adding assertions for you.