[HN Gopher] Can sanitizers find the two bugs I wrote in C++?
___________________________________________________________________
Can sanitizers find the two bugs I wrote in C++?
Author : signa11
Score : 122 points
Date : 2023-02-08 09:42 UTC (13 hours ago)
(HTM) web link (ahelwer.ca)
(TXT) w3m dump (ahelwer.ca)
| viraptor wrote:
| I wonder if PVS would flag those issues. (https://pvs-
| studio.com/en/) I expect it to flag the atoi, but not sure about
| the pop.
| wyldfire wrote:
| Another option for dynamic bounds checking on heap allocations is
| Electric Fence or scudo. It probably wouldn't find that issue
| with pop_front() though.
| cjbprime wrote:
| Nice quote from the previous post in this series:
|
| > 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++.
| jancsika wrote:
| C developers: just don't reach back into the bag. It's pretty
| simple.yyyy
| BoorishBears wrote:
| ChatGPT finds the bugs...
|
| -
|
| 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._
| ynik wrote:
| asan/ubsan are great for finding language-level undefined
| behavior. But apart from some functions like memcpy that asan is
| hooking, they cannot find library-level undefined behavior such
| as the pop_back() on the empty vector. (That's an inline function
| inside a template, there's no way asan could hook this!)
|
| 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)
| roel_v wrote:
| Meta: when you ask chatgpt what happens when you do pop_back()
| on an empty vector, it confidently claims std::out_of_range
| will be thrown. It will even 'cite' the standard with a
| plausible explanation of what will happen (essentially, it says
| that a prereq is that empty() == false, and that it depends on
| what erase() throws).
| https://cplusplus.com/reference/vector/vector/pop_back/ and
| other sources I can find say undefined behavior. I don't have a
| copy of the standard at hand, can anyone quote the relevant
| section? Is this something that in practice is implemented in
| different (exception-throwing) ways?
| dpkirchner wrote:
| When I asked ChatGPT "In C++, what happens when you call
| pop_back() on an empty vector?" I got the answer:
|
| > 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.
| roel_v wrote:
| Here's my exact chat log: 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.
| aw1621107 wrote:
| Pretty interesting output. Some notes from a cursory
| glance/crosscheck against the first post-C++20 draft [0]:
|
| 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...
|
| [1]: https://www.iso.org/standard/79358.html
| roel_v wrote:
| Interestingly, here's what I get when I feed it your exact
| question: 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.
|
| So it basically claims both that an exception is thrown,
| and that it's undefined behavior. At least in this case,
| one would know something is up.
| dpkirchner wrote:
| I asked it again (in a new context) and got a partially
| right answer as well:
|
| > 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?
| gpderetta wrote:
| > Is this something that in practice is implemented in
| different (exception-throwing) ways?
|
| I don't think so. Even when enabling debugging mode, a
| precondition failure would cause an abort, not an exception
| to be thrown.
| saurik wrote:
| God help us if people start trying to use an overconfident
| chatbot to replace documentation :(.
| SketchySeaBeast wrote:
| That would very much be the Warhammer 40k machine spirit -
| the operator would only do what they can to appease it, and
| even the spirit wouldn't know how or why something did or
| didn't work.
| simplotek wrote:
| > God help us if people start trying to use an
| overconfident chatbot to replace documentation :(.
|
| 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.
| imtringued wrote:
| Here is the documentation that you consider unclear
|
| >If the container is not empty, the function never throws
| exceptions (no-throw guarantee). Otherwise, it causes
| undefined behavior.
|
| ChatGPT failed at basic regurgitation.
| tialaramex wrote:
| This is, at best, clumsily worded, because it requires
| that the reader hold two negatives simultaneously. "If
| the container is _not_ empty, the function _never_ throws
| "
|
| 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.
| chipsa wrote:
| The function never throws, whether empty or not. So, skip
| the "otherwise".
| tialaramex wrote:
| It's not called "Undefined except we promise it can't
| possibly throw" behaviour.
| geraneum wrote:
| > If the chatbot built around the docs outputs garbage,
| that is a telltale sign that the documentation was
| garbage to start with.
|
| 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
| ziml77 wrote:
| Poor output from a chat bot indicating poor documentation
| would make sense if the chat bot could limit itself to
| authoritative information from the documentation, but
| that's not how these models operate. I'm not even sure
| how you could make them work that way because if you
| trained them solely on the documentation, they wouldn't
| be able to operate in a conversational manner.
| imtringued wrote:
| You can tune ChatGPT with custom data.
| saurik wrote:
| This is, AFAIK, not a task for fine tuning--which is more
| like trying to get a certain kind of style-but maybe a
| task for "semantic search". I watched this video a few
| days ago, and at least vaguely recommend it:
| https://youtu.be/9qq6HTr7Ocw
| saagarjha wrote:
| I mean it's fine if you can double-check what it tells you
| quickly, it's just pretty bad with something like C++
| because when it gives you something wrong the program may
| or may not tell you about it. For example, what it might be
| good for is "what are the parameters to this 6-argument
| function that plots a rectangle on the screen", because
| that's something you can confirm really easily and there
| are no real consequences to getting it wrong. On the other
| hand, someone sent me a screenshot of how ChatGPT was
| really helpful in figuring out how reader-writer locks
| work, and it told them "you can use
| pthread_rwlock_trywrlock to upgrade a reader lock to a
| writer lock" :/
| roel_v wrote:
| Maybe. To be fair, I had the same opinion yesterday. This
| morning I decided to take a real look at how I could use
| chatgpt to augment my programming, and once I started with
| it with an open mind, I now think it's going to take away
| much of the drudgery I have come to loathe so much about
| programming. I used to like delving into libraries and
| frameworks and learning all their details - I thought that
| was what 'learning about programming' was. Now, 20 years
| later, I hate those aspects with a passion - having to look
| up the idiosyncrasies of a function call _again_ , because
| there are dozens of ways of using it but all slightly
| different, or piecing together another dull specific use
| case. This morning I had chatgpt generate 4 fully fledged
| functions for me, each doing a highly specific task that
| would have taken maybe 20 or 30 minutes each to piece
| together the specific API's, and one of them using a Python
| package I had never used before, didn't know of and would
| have had to read the getting started tutorial before I
| could have used it on my own. Then all I had to do was
| write the glue to call those functions in the right order
| and with the right arguments and boom that was it. Plus the
| code was more elaborate and coherent than it would have
| been had I have to do it manually.
|
| 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 :)
| Someone wrote:
| > _This morning I had chatgpt generate 4 fully fledged
| functions for me, each doing a highly specific task that
| would have taken maybe 20 or 30 minutes each to piece
| together the specific API 's, and one of them using a
| Python package I had never used before, didn't know of
| and would have had to read the getting started tutorial
| before I could have used it on my own. Then all I had to
| do was write the glue to call those functions in the
| right order and with the right arguments and boom that
| was it._
|
| "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.
| wizofaus wrote:
| Can it also generate unit tests for you to prove the
| functions work as expected? Obviously those unit tests
| need to be something a quick glance at will confirm are
| actually usefully verifying expected behaviour (of
| course, there are circumstances that's nigh on impossible
| to do even with the most carefully handcrafted tests).
| [deleted]
| hobs wrote:
| https://supabase.com/blog/chatgpt-supabase-docs Already on
| HN
| williamcotton wrote:
| The technique they're using is much less error prone due
| to the nature of the prompt.
|
| 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.
| taneq wrote:
| Doesn't see any worse than asking HN. :P
| drewcoo wrote:
| Chatbot moves in mysterious ways.
| JulianMorrison wrote:
| They're using it to replace "search on stack overflow, cut
| and paste, does it work?"
| bigbillheck wrote:
| > if
|
| * when
| aw1621107 wrote:
| > I don't have a copy of the standard at hand, can anyone
| quote the relevant section?
|
| 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%2
| B-v3...
| jb1991 wrote:
| > Compiling it needs Perl
|
| 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.
| Karellen wrote:
| You've just been pointed at github. Which hosts git
| repositories. Which is used to keep track of changes to
| _source files_ , not published build artifacts.
|
| 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.
| dminik wrote:
| You do know that GitHub has an entire section for each
| repository called "Releases" right? In fact, you can
| setup a free pipeline to generate these release for you.
|
| Might be worth keeping in mind if you ever need to
| interact with git (or github) again.
| aw1621107 wrote:
| The C++ standard is a LaTeX document, not a plain text
| document. Perl is needed for latexmk [0], which isn't
| strictly necessary but greatly simplifies the process of
| compiling LaTeX.
|
| [0]: https://mg.readthedocs.io/latexmk.html
| elteto wrote:
| The final standard document is neither a "text" document,
| nor do you need Perl to "read" it.
| planede wrote:
| Current C++ standard draft it hosted at
| https://eel.is/c++draft/, at this time this is the draft for
| C++23.
|
| 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.
| WalterBright wrote:
| > As a C++ language reference I highly recommend
| https://en.cppreference.com
|
| 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.
| mattgreenrocks wrote:
| I'm guessing that ChatGPT answered the query based off of
| implementation-specific details, rather than the standard
| itself.
|
| There's a dark irony in the fact that programmers do this
| _all_ the time.
| planede wrote:
| No implementation that I know of throws an exception
| here.
| mattgreenrocks wrote:
| I stand corrected. It sounded like something that would
| be done in debug mode by some STL implementations.
| planede wrote:
| In debug mode libstdc++ and libc++ aborts the program
| with a diagnostic message. This should not be an
| exception that can be caught, and it rightfully isn't.
| pjmlp wrote:
| They aren't the only implementations around.
| AnimalMuppet wrote:
| So it's ChatGPT confidently making stuff up. Lovely.
| mr_00ff00 wrote:
| Is there a reason some sort of check wasn't added to
| pop_back() implementation?
|
| Seems like it would be easy to implement and undefined
| behavior is not something that backwards compatibility
| forces you to keep.
| ynik wrote:
| pop_back() implementations have checks, but they're opt-
| in (and how to opt-in is specific to each
| implementation).
|
| 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.
| asveikau wrote:
| The standard does not require it.
|
| 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.
| wizofaus wrote:
| Because the basic principle of C/C++ is to not force the
| coder to have to pay for checks it mightn't need. Which
| in some contexts makes sense - for application
| development, probably not.
| fooker wrote:
| pop_back() is internally just changing a single index.
|
| 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.
| WalterBright wrote:
| > when you ask chatgpt what happens
|
| 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.
| gpderetta wrote:
| Also, it did still catch a problem in the second scenario, it
| just didn't conveniently point to the right line. +1 on
| _GLIBXCXX_{ASSERTION,DEBUG}.
| gfd wrote:
| According to https://gcc.gnu.org/onlinedocs/libstdc++/manual/
| debug_mode_u... there's a D_GLIBCXX_DEBUG_BACKTRACE that will
| show a backtrace for the line number too
| Ygg2 wrote:
| As someone coming from Java, why is pop_back undefined
| behavior?
|
| An error or noop is something I expect, but undefined behavior?
| planede wrote:
| pop_back on an empty container is a programmer error.
|
| 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.
| dannymi wrote:
| It's C++. Its goal is speed over all else. So it tries to
| carve out space for where the optimizer can assume lots of
| things are true. Since they don't use anything like Hoare
| logic they have to get their assertions from somewhere else,
| inferring them as follows (among other things):
|
| 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.
| kllrnohj wrote:
| > 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.
|
| 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)
| tialaramex wrote:
| I had once assumed std::unordered_map was from the
| original Standard Template Library (which pre-dates C++
| standardization), but turns out (I was told in a r/cpp
| thread) it's more modern. In 1998 std::unordered_map
| would have been an excusable design mistake, domain
| experts knew in 1998 that's a sub-optimal design for this
| dict / hashmap structure - but you could suppose perhaps
| they were not on the committee. In 2011 it's not an
| excusable mistake, better structures are widely known in
| academic and in industry - it was an unforced error.
|
| 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.
| gpderetta wrote:
| hash_map was indeed part of the original STL, but was not
| standardized in C++98. It was still widely available, in
| slightly incompatible forms, as an extension. In C++11 it
| was standardized with only minor changes (and renamed).
|
| The address-preserving property, which precludes an
| optimal implementation, was considered important to be a
| near drop-in replacement of std::map.
| pjmlp wrote:
| Yet we get stuff like std::regex, the speeding train of
| regular expression parser engines.
| jcelerier wrote:
| I don't really see how one could make it a real no-op, and an
| error would imply that pop_back would look like
| if(empty()) { /* some error handling mechanism */ }
| ... rest of the code ...
|
| and every added branch adds some minimal but non zero cost
| tialaramex wrote:
| However _really often_ this is a case you actually care
| about, and so if you don 't capture it in pop you end up
| asking a predicate. I have a whole bunch of Rust code that
| does: while let Some(thing) =
| container.pop() { /* ... */ }
|
| There is indeed a branch, that pop function might return
| None - but when it does our while loop exits, the container
| is empty and we're done. My guess is that the idiomatic C++
| here checks !empty() for the loop and then calls pop() on
| the container if it's not empty so in these cases we're
| doing the same work except we made it more error prone.
| dezgeg wrote:
| It presumably allows the implementation to be a tiny bit
| faster, by avoiding a compare and branch for the empty case.
| taneq wrote:
| Because unlike Java, which is designed to bore you to death,
| C++ is designed to gaslight you into madness.
| dureuill wrote:
| I had more bugs pop up from people calling front() on an
| empty vector than from direct out of range bounds. I guess
| people are more cautious when they are manipulating indices
| than when calling a method that takes 0 arguments?
|
| 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.
| planede wrote:
| Well, front() returns a reference. std::optional can't hold
| references. The closest you get is a plain pointer, with a
| null pointer for an empty container.
| dureuill wrote:
| Other languages allow to return `Option<&T>`. That's what
| I'm using these days.
|
| I suppose a raw ptr is what I would use on C++ to model
| that case though, yes.
| planede wrote:
| Yeah, for some reason `std::optional<T&>` is cursed and
| can't get through standardization. People can't decide
| what should happen on assignment.
| dureuill wrote:
| I guess a raw pointer is good enough an approximation in
| that case, it is just annoying that suddenly you can no
| longer use an optional because your type is a reference
| st_goliath wrote:
| I suppose it causes an integer underflow _and_ tryies to in-
| place destruct in an empty buffer?
|
| 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.
| gpderetta wrote:
| STL was designed to have no overhead over the corresponding
| C code. If push_back (and all other functions) had an extra
| check even when it was guaranteed non-empty by
| construction, it would have been a non-starter for many
| users.
|
| 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.
| simiones wrote:
| This makes sense for std::vector::operator[], but not for
| pop_back and push_back - as those don't have any
| equivalent for a C array. Instead, it's just another
| example of C++ optimizing for speed as opposed to
| correctness.
| gpderetta wrote:
| The equivalent C code is what you would write inline to
| do a pop_back on an array. You can't write a fast
| pop_back for std::vector if it didn't provide one by
| default as the fields you would need to manipulate are
| private.
|
| And yes, it is indeed an instance of C++ optimizing for
| speed, which is exactly what I said.
| simiones wrote:
| std::vector exposes the exact same API as a C array, in
| terms of being able to write an external pop_back()
| function, since C arrays only expose the ability to
| access a particular element - no more, no less (and
| std::vector::resize() can be used instead of realloc() if
| that is desired).
| gpderetta wrote:
| The equivalent of of std vector is an array+size+capacity
| field. pop_back would litterally be capacity--. resize
| has overhead as it needs to handle resize up as well as
| down.
| usefulcat wrote:
| > pop_back would litterally be capacity--.
|
| Minor nit: I think you meant 'size--'
| gpderetta wrote:
| ouch, yes of course
| simiones wrote:
| Sure, and I'm saying in C you could write:
| struct stack { int[] elems; int len, cap;
| }; int pop_back(struct stack *x) {
| elem = x->elems[x->len - 1]; x->len--;
| return elem; }
|
| while in C++ you could have written:
| struct stack { std::vector elems; int
| len, cap; }; int pop_back(stack& x) {
| elem = x[x.len - 1]; x.len--; return
| elem; }
|
| If you wanted C-style speed, with no safety.
| zabzonk wrote:
| that is not valid c. and in valid c++ (which this isn't)
| you would not need those two integer values.
| tsimionescu wrote:
| There are some minor syntax errors - I should have
| written `int *elems` instead of `int[] elems`, and I
| forgot to declare `int elem` instead. Apart from these
| nitpicks, it's completely valid C.
|
| 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.
| Liquid_Fire wrote:
| Java is memory safe, at a runtime cost (sometimes that cost
| is trivial, sometimes it isn't). It will insert a check every
| time you do a pop.
|
| 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.
| zabzonk wrote:
| typically, c++ library functions do not use a possibly
| expensive check if the caller could have performed it
| themselves, should they have wished to. in this case, the
| caller could have tested if the container was empty before
| calling the function.
| tubszonk wrote:
| Because in c++ we assume the programmer doesn't make logic
| mistakes, and optimize for speed instead. A novice programmer
| can add assert() as needed.
| Ygg2 wrote:
| To use a car analogy - We removed seatbelts to go faster,
| just don't make mistakes while driving.
| spoiler wrote:
| I'm not a novice programmer (close to 15 year in total) and
| I still make mistakes.
|
| -\\_(tsu)_/- Maybe I'm just stupid
| jcelerier wrote:
| > asan/ubsan are great for finding language-level undefined
| behavior. But apart from some functions like memcpy that asan
| is hooking, they cannot find library-level undefined behavior
| such as the pop_back() on the empty vector. (That's an inline
| function inside a template, there's no way asan could hook
| this!)
|
| 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)
| ynik wrote:
| That's a special case due to an empty vector using nullptr,
| and thus causing language-level UB when doing pointer
| arithmetic with a nullptr.
|
| 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
| jcelerier wrote:
| ahhh just double checked and it is hidden behind a macro
| for libstdc++... see: https://gcc.godbolt.org/z/e7Wj163PM
| ynik wrote:
| Interesting, I wasn't aware of that one. I think
| `-D_GLIBCXX_ASSERTIONS` is more appropriate for catching
| this bug, but `-D_GLIBCXX_SANITIZE_VECTOR` seems useful
| for cases like this: https://gcc.godbolt.org/z/EjrdoTYPs
|
| 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.
| simiones wrote:
| So, if following the recommended C++ practices of mostly using
| std containers, ASan and UBSan are mostly useless?
| synergy20 wrote:
| containers are not thread safe though, so the thread safe
| sanitizer will be useful there.
|
| 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.
| planede wrote:
| AFAIK `_GLIBCXX_DEBUG` alters ABI to the library, it should be
| only used with a libstdc++ and all other components compiled
| with the same flag. Debug iterators, and all that jazz. MSVC
| users are probably not surprised.
|
| `_GLIBCXX_ASSERTIONS` does not alter ABI.
| wyldfire wrote:
| From https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_ma
| cros...
|
| > 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.
| gpderetta wrote:
| very technically defining _GLIBCXX_DEBUG in different ways
| across translation units violates the ODR, so it is UB. In
| practice this is expected to work with GCC[1] and the and
| the macro is ABI safe.
|
| [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.
| planede wrote:
| Well, if we are nitpicking here, defining
| _GLIBCXX_ASSERTIONS in different ways across translation
| units also potentially violates ODR, as some inline
| functions or function templates get different definitions
| in different translation units.
|
| 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.
| planede wrote:
| It's not documented well.
|
| 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.
| zabzonk wrote:
| well, one of the problems could have been detected with "grep
| atoi source_code_here". any code that uses atoi is almost
| certainly bad (no error detection), and C++ provides several
| alternatives.
| jeffbee wrote:
| Right? Why do people love these C library functions? I think
| the biggest problem in the C++ community is that some people
| know a little C.
|
| 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.
| eklitzke wrote:
| In this case the code was using the .data() on a
| std::vector<char>, so it wouldn't have even been possible to
| use something like std::stoi (or something nicer like
| absl::SimpleAtoi) since the C++ routines need a std::string
| (or absl will accept std::string_view), not a C style string.
| Which in itself shows you that the code is problematic,
| because it's been written in a way where you can't even use
| idiomatic C++ library functions to do basic tasks.
|
| 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().
| gpderetta wrote:
| Ideally the compiler would suggest to replace atoi with the
| C++ equivalent. But not even clang-tidy does with the set of
| warnings I have enabled.
| pclmulqdq wrote:
| That's not the philosophy of the C++ compiler writers.
| There are linters that suggest it, but compilers don't do
| this sort of warning.
| zabzonk wrote:
| i don't even understand why atoi gets used in pure C code -
| there are much better standard C alternatives.
| jussij wrote:
| This is the beauty of modern C++. The code has become so hard to
| read (without years of prior knowledge) that as a human, it's
| rather hard to find any possible errors. The code can take on a
| Schrodinger state where it appears both right and wrong.
| jeroenhd wrote:
| I don't think it's C++'s fault necessarily. The C/C++ ecosystem
| seems to attract more of a certain type of developer who will
| tell you to just "get good" at the language so you van decipher
| their code and use it without a billion memory bugs, but the
| language doesn't specify any of it.
|
| 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.
| pjmlp wrote:
| Modern tooling that usually only 11% of the community that
| bothers to answer surveys uses.
| simplotek wrote:
| The code is not hard to read.
|
| 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.
| bArray wrote:
| I don't not think that English isn't difficult to parse.
|
| 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.
|
| [1] https://github.com/rust-lang/rust/issues
| galangalalgol wrote:
| Rust doesn't create any new classes of bugs, and it
| eliminates some old ones. Isn't that all positive? Rust
| also turns a lot more bugs from UB and/or exploitable, into
| a panic. That is still potentially a ddos, but it is better
| than code injection. And therebare tools like prusti to
| find panics, and you can use tricks with linking a nostd
| library to see if you got them all. Probably better ways
| than that now.
|
| 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)
| kllrnohj wrote:
| > Rust also turns a lot more bugs from UB and/or
| exploitable, into a panic.
|
| 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.
| hra5th wrote:
| Rust integer overflow bugs in release mode are still much
| safer than C++ integer overflows -- Rust integer overflow
| is well-defined to wrap in release mode, whereas it is UB
| in C++.
| kllrnohj wrote:
| That's not meaningfully safer which is why it's still a
| panic in debug builds. It's really just kinda worse even.
| You can't use it as a programmer (because it panics) and
| the compiler can't use it even though you've already
| promised (and debug mode verified) that it never happens.
| galangalalgol wrote:
| If you want an add to wrap, you should use a wrapped_add,
| useful for angle math or whatever. If you want it to
| saturate, use a saturating_add, and if you want to check
| for overflow, use a checked_add. If I were to write a
| rust coding standard it would prohibit + in favor of
| explicitly using those functions.
| kilgnad wrote:
| Categorically not true.
|
| 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.
| ksherlock wrote:
| 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; }
|
| eek. i've always done something like: level =
| std::accumulate(raw_level.begin(), raw_level.end(), 0,
| [](int a, int c) { return a * 10 + (c - '0') } );
|
| How does everyone else atoi when you can't atoi (and don't care
| about overflow etc)
| batty_alex wrote:
| > for (size_t i = 0; i < raw_level.size(); i++)
|
| 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
| mr_00ff00 wrote:
| As this is HN, all trendy systems languages must be mentioned
| so here I go:
|
| 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.
| ddulaney wrote:
| Unfortunately, it's _also_ C++ code. Most of the weakest parts
| of C++ come from its close association with C.
|
| 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
| omnicognate wrote:
| Enabling libstdc++ debug mode (-D_GLIBCXX_DEBUG) would probably
| have located the second one, assuming gcc. It adds bounds and
| other checks to containers like std::vector. [1]
|
| Edit: The article mentions clang, so probably [2] is the relevant
| link.
|
| [1]
| https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_u...
|
| [2] https://libcxx.llvm.org/DesignDocs/DebugMode.html
| account42 wrote:
| You can use libstdc++ with Clang (-stdlib=libstdc++) and libc++
| with GCC (-nostdlib along with adding the libraries and include
| paths manually). In fact, libstdc++ tends to be the default for
| Clang installs on Linux for ABI-compatibility with GCC-compiled
| C++ libraries unless you pass -stdlib=libc++.
| inetknght wrote:
| This post supports my personal experience. ASAN is a flippin'
| godsend. I have _never_ encountered a false report from ASAN. I
| have seen it have false negatives but those are usually
| intermittent with true positives when the tests are run again and
| usually related to very small buffer overflows (eg, a "few
| bytes" past the end of a buffer). Not perfect but no false
| positives means it's 100% worthwhile to have enabled.
|
| 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
| mannykannot wrote:
| With regard to UBSAN, are the misses that you see examples of
| specific checkers failing to find the specific sort of UB that
| they are intended to detect? If so, do the majority of misses
| seem to come from a small subset of the checkers?
| cyber_kinetist wrote:
| I think there's a place for Address Sanitizer to be enabled on
| RelWithDbgInfo build type (usually used for development and not
| production). Debug mode in C++ is already too slow, but adding
| a sanitizer makes it almost unusable for performance-sensitive
| code.
| kllrnohj wrote:
| The sanitizers are not incompatible with optimizations. asan
| with -O2 is a perfectly valid & useful combination. It's
| _much_ faster than valgrind or anything like that, very
| usable levels of performance. And on an increasingly common
| number of CPUs there 's hardware acceleration for ASAN as
| well ( https://clang.llvm.org/docs/HardwareAssistedAddressSan
| itizer... &
| https://source.android.com/docs/security/test/hwasan )
| gpderetta wrote:
| The sanitizer are all great! I've also had TSAN find non-
| trivial issues with my threaded code.
|
| 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).
| brandmeyer wrote:
| Running multiple test builds in parallel isn't all that
| difficult, though. One with ubsan, one with asan, and one (opt-
| in on a test-by-test basis) with tsan.
| eklitzke wrote:
| I don't understand the comment in the post about how the vector
| including a large address space after .pop_back() could include
| private keys.
|
| 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.
| cjfd wrote:
| Well, being a C++ programmer for years who really would not mind
| going back to C++. Yes, these are known things. When one wants to
| call atoi, one has to know that either the string is zero
| terminated or that non-numerical characters follow the numerical
| ones... Same kind of thing with pop_back. I would not know that
| calling pop_back on an empty container is undefined behaviour by
| heart but I would be suspicious enough that it might be to check
| the docs to find out.
| roelschroeven wrote:
| Putting string data in a vector<char> for starters is a big
| code smell. If you do, you're not using the language and the
| standard library the way they are supposed to be used and it's
| on you to make sure you do everything right.
|
| Then combining that with C-style string handling functions,
| without taking all the extra care, is just asking for trouble
| IMO.
| axilmar wrote:
| The problem of hidden types surfaces again.
|
| 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 {
| ... }
|
| But this is not enough. The language should allow the user to
| specify change of states at certain statements/expressions. For
| example: void clear() { ...
| this := empty; }
|
| And then the compiler can check the AST if the used vector has
| the 'empty' state on pop back or not. Doing the following would
| be illegal: vector<int> data;
| data.clear(); data.pop_back();
|
| And the following would be illegal because one execution path
| would lead to a state violation: vector<int>
| data; if (some_external_condition) {
| data.clear(); } data.pop_back();
|
| And the following would be illegal too, because the result of
| clear_random_vector could be possible in empty state.
| 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; }
|
| int main() { vector<int> a{1, 2, 3}, b{4, 5, 6}; vector<int>&v =
| clear_random_vector(a, b); v.pop_back(); }
|
| 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;
|
| b) if modules are used, the compiler itself should output the
| metadata.
|
| 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);
|
| And then this would be caught at compile time:
| std::vector<char> dst, src; memcpy(dst.data(),
| src.data(), 10);
|
| States could also solve the problem of array indexing. For
| example, an index can have two states, 'in_range' and
| 'out_of_range'. Using an integer as an array index would
| implicitly add the two states in the index variable; comparing
| the integer with the array range would automatically add
| 'in_range' or 'out_of_range' to the integer.
|
| 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; }
|
| The following would be invalid, because straightforward array
| indexing would requre a state named 'in-range':
| vector<int> data{1, 2, 3}; int i = 0; //our index
| std::cout << data[i] << std::endl;
|
| How would that work at language level? well, here is an idea: a)
| any operation can affect the compile-time state of objects, b)
| states themselves would be templates, bound to certain ids at
| compile time.
|
| 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>;
| } }
|
| So, each time push_back is called on a vector, the guarantee
| about indexes breaks, and the index that previously holds the
| guarrantee can no longer be used for indexing.
|
| 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 ;-).
| [deleted]
| aw1621107 wrote:
| I think what you're describing is basically dependent types
| [0], where types can depend on values.
|
| [0]: https://en.wikipedia.org/wiki/Dependent_type
| zabzonk wrote:
| by your logic, a vector with 3 elements is a different type to
| a vector with 4 elements. this is where original pascal
| famously went off the rails
| ThatGeoGuy wrote:
| Which starts to look an awful lot like dependent types.
| Unfortunately any sane use-case of that in 2023 looks like
| Idris or Coq and nobody in C++ today is switching to either
| of those.
| TinkersW wrote:
| The pop_back() error would have been caught easily by an
| experienced C++ dev as common sense says run with assertions
| enabled in dev builds(With MSVC use _CONTAINER_DEBUG_LEVEL =1).
|
| The other error was just dumb using a C function when a good C++
| alternative exists.
| Kranar wrote:
| Don't know what your domain is, but most experienced devs in
| most of the domains I work in turn off those assertions because
| they slow the absolute hell out of debug builds.
|
| 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.
| TinkersW wrote:
| No they don't, you are thinking of older implementations(the
| old checked iterators _ITERATOR_DEBUG_LEVEL etc in msvc).
| That shit was indeed very slow.
|
| You can run release builds with _CONTAINER_DEBUG_LEVEL =1 and
| the performance impact is minor.
| pornel wrote:
| Is there a C++ proposal to add mandatory bounds checks to methods
| like pop_back? If modern C++ is serious about getting memory
| safety, UB like this should be changed to have a run-time check
| and abort.
|
| 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.
| aw1621107 wrote:
| At least based on this comment from the maintainer of the C++
| NanoRanges library [0], it seems contracts (at one point) may
| be the intended method to bounds check:
|
| > 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_...
| gpderetta wrote:
| There is a deep divide between those that believe that
| contract violations should never result in exceptions and
| those that believe that they should. In practice we end up
| with the worst of both worlds were we get inconsistent
| semantics on specific APIs depending on who's present on the
| day the feature is voted in (hence _at_ for vector but not in
| span), while large changes like contracts never reach
| consensus.
|
| This is more and more of an issue with the c++
| standardization process.
| bluGill wrote:
| I'm not sure what the current thought is, but I'm hopeful
| contracts makes it into C++, and over a the next years (and
| iterations of the language) we find ways to make C++ better
| by proving the whole program correct. This will need a lot of
| tool support, and changes in the way people write code.
| However mathematical proofs of the program are the only thing
| I have left to make my code more reliable: humans are not
| perfect and unit tests have limits (proofs have different
| limits).
| WanderPanda wrote:
| I don't entirely get the concept between mathematical
| proofs of programs. Is the basic idea that it is straight
| forward for humands to write down a correct NP hard
| specification and then let some algorithm proof the
| correctness of a more efficient algorithm for the same
| problem?
| bluGill wrote:
| It isn't straight forward. It is claimed to be possible
| and useful by advocates.
| pjmlp wrote:
| Keep hoping then, it looks like it won't make C++26 as
| well.
| bluGill wrote:
| Bounds checking is not zero cost in all cases. So long as you
| can demonstrate even one example of a case where the bounds
| checking isn't zero cost, and yet you can prove that the bounds
| check isn't needed there will be push back. C++ is about don't
| pay for what you don't use, and if you don't need the bounds
| check you shouldn't have to pay for it. You cannot have high
| performance programing if there are various 'little' if
| statements added all over.
|
| 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.
| oconnor663 wrote:
| > C++ is about don't pay for what you don't use, and if you
| don't need the bounds check you shouldn't have to pay for it.
|
| 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/hsu
| tter/cppfront/issues/110#issuecomment-... )
| kllrnohj wrote:
| pop_back is about to invoke a destructor. Remember unlike
| what the name would suggest, it's not actually returning the
| element it removed - it's equivalent to erase(end() - 1). So
| adding a bounds check is going to be inconsequential in
| nearly any usage. And if you're doing a lot of pop_backs all
| at once, then if you cared about Real Speed you'd obviously
| be using the range erase() to do it all in one go.
| pjmlp wrote:
| Zero cost means it won't be slower if I had to manually write
| the bounds checking myself.
|
| > 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.
| pornel wrote:
| I keep hearing how Modern C++ is safe now, and all the dumb
| UB was just C and legacy unidiomatic C++ code (even recently
| argued by Bjarne). But std::vector isn't legacy, has easy UB,
| and there's strong pushback like yours against fixing it.
|
| I don't see Modern C++ ever getting past its safety-third
| design.
| kllrnohj wrote:
| > But std::vector isn't legacy
|
| 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.
| dureuill wrote:
| I don't follow. What should one use to have safe and
| efficient dynamically growable arrays in modern C++ if
| `std::vector` is not considered as modern?
| pornel wrote:
| You can't be serious. "Don't use std::vector" is the
| modern C++ recommendation now?
|
| If it is, it should throw a warning that it's deprecated
| and have a replacement for it.
| kllrnohj wrote:
| I didn't say it wasn't recommended or should be avoided,
| but it definitely has "legacy" to it. It's not an
| exclusively "Modern C++" container.
| SleepyMyroslav wrote:
| if you are interested in C++ usage ... then you might
| learn that vector is the only std container that is
| relevant in modern context.
|
| 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.
| kllrnohj wrote:
| I use vector all the time, but that doesn't mean it's
| without any legacy baggage. It didn't get a clean sheet
| reboot in "Modern C++"
|
| 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.
| bluGill wrote:
| Tradeoffs. Everyone has to make them. Modern c++ is safe in
| the mistakes people often make. Sure there is unsafe stuff,
| but they are not in things where you would normally do
| that.
| jkbbwr wrote:
| You could potentially hit your codebase with CBMC using model
| assertions.
|
| Its basically what you already know in the TLA+ world but
| targeted against C++ code.
| meindnoch wrote:
| Don't use atoi. Use std::from_chars.
| simiones wrote:
| Is there an alternative if not using C++17 or newer?
| zabzonk wrote:
| several (or should that be many?) - for example stoi, sscanf,
| operator >>(istream &, int &) - there are others.
| account42 wrote:
| This makes sense for integers but beware floating point
| from_chars - libc++ still doesn't implement it and libstdc++
| implements it by wrapping locale-dependent libc functions which
| involves temporarily changing the thread locale and possibly
| memory allocation to make the passed string 0-terminated. IMO
| libstdc++'s checkbox "solution" is worse than not implementing
| it at all - user's are better off using Lemire's API-compatible
| fast_float implementation [0].
|
| 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.
|
| [0] https://github.com/fastfloat/fast_float
| meindnoch wrote:
| Hm. Good to know! What about strtof?
| zabzonk wrote:
| the strtoX functions have been around for many, many years,
| and should be completely reliable. but i would use strtod.
| in fact i never use floats in my code unless i am dealing
| with very large numbers of floating-point values.
| vitus wrote:
| The actual bug was using .data() instead of .c_str() when the
| author needed a C-style string.
|
| But yes, avoiding error-prone functions is an equally effective
| approach.
| zabzonk wrote:
| c_str and data are now defined to be identical - both return
| a null-terminated array of char
| HarHarVeryFunny wrote:
| I don't think std::vector<char> supports c_str() - presumably
| you're thinking of std::string ?
|
| Of course he could have done:
|
| atoi(std::string(vec.data(), vec.size()).c_str());
|
| But I guess he wanted bugs to test against.
| HarHarVeryFunny wrote:
| Actually just realized these were for-real bugs, not just
| sanitizer testing.
|
| 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!
| gpderetta wrote:
| Are you confusing the sanitizers with static analyzers?
|
| 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.
| HarHarVeryFunny wrote:
| You're right - was referring to things like Fortify.
| tubs wrote:
| LD_PRELOAD is probably want you want.
___________________________________________________________________
(page generated 2023-02-08 23:01 UTC)