CppCon 2017: Louis Brandy “Curiously Recurring C++ Bugs at Facebook”
Articles Blog

CppCon 2017: Louis Brandy “Curiously Recurring C++ Bugs at Facebook”


41 thoughts on “CppCon 2017: Louis Brandy “Curiously Recurring C++ Bugs at Facebook”

  1. The std::string/unique_lock issue(32:20), while it blew my mind that this happens 🙂 – it's weird that people wrote that code anyway, it is an rvalue and even if that code would of worked, the destructor is instantly called after the constructor.

  2. Constructing an object using: Type(var); to declare a variable "var" of type "Type" should just never existed. No one use it anymore except bug makers…
    This is confusing for everyone: developers, compilers (make them more complex), any other parsing tools.
    Hope future C++ standards will remove it the same way as trigraph.

  3. 10:24 That's called "cargo cult programming" and in my opinion it's the first source of bugs made by newbies in any language. 😉

  4. 12:18 I think a method like vector<T>::value_or (T const&, T fallback_value) const; could be useful if added to the standard. 😉 Or maybe just add "default" value parameter to the ::at() method?

  5. for the map square bracket operator. perhaps returning some form of std::optional? not sure how well that plays into performance critical or clean looking code tho 😀

  6. "and then think a little bit more existentially, like what are we doing, and why are we here? I feel like I'm wasting my life a little….ok!" @30:43 lol!

  7. How can you say "shared_ptr isn't slow" and then explain that the control block access is thread-safe? It doesn't show up in your profile because the code all gets inlined, but of course it's slower than using a bare pointer because it's doing a bunch of extra work every time you copy it or let it go out of scope.

  8. Thank you very much for this talk. It again shows how difficult it is to solve easy problems like returning some value from a method is in C++. C++ is too much performance-centered instead of keeping it easy to read and write.

  9. 27:30 – is atomic<shared_ptr> really correct? I thought the template argument of std::atomic has to be trivially-copyable?

  10. STL is crap. It's just a bunch of weird things put together. Most of the time you spend a long time trying to understand how it is implemented, just to find out that it was implemented in the strangest possible way. No language additions or new abstraction could make it safer.

  11. If it's a class method, you can get around the RAII initialization bug by always using "this->" before member variables 🙂

  12. Why does he say that "shared_ptr isn't thread safe" is technically a lie?
    If only the control block is thread safe then why isn't this an absolute no?

  13. Regarding Bug #3:
    The solution seemed obvious to me… detect if the default value is a temporary, and either disallow it or make that version return by copy.
    I went and looked it up in the folly library, and at least now it seem they do exactly that. except they call that get_ref_default() instead:

    template <typename Map, typename Key>
    typename Map::mapped_type get_default(const Map& map, const Key& key);

    template <class Map, typename Key = typename Map::key_type>
    const typename Map::mapped_type& get_ref_default(const Map& map, const Key& key, const typename Map::mapped_type& dflt);

    template <class Map, typename Key = typename Map::key_type>
    const typename Map::mapped_type& get_ref_default(const Map& map, const Key& key, typename Map::mapped_type&& dflt) = delete;

    template <class Map, typename Key = typename Map::key_type>
    const typename Map::mapped_type& get_ref_default(const Map& map, const Key& key, const typename Map::mapped_type&& dflt) = delete;

  14. Guys at my job wouldn't allow using non initialized consts, like map["key"]. We always initialize them, because it won't compile if you make a typo.

  15. at 32:15 alert("spoiler!");

    lock_guard<mutex>(m_mutex); // this would still cause problems
    do_the_mutation();

    -> the unnamed temporary's ~lock_guard() would release mutex prior to do_the_mutation
    -> nothing gets synchronized

    You could fix it right away:

    lock_guard<mutex>(m_mutex), do_the_mutation();

    -> demonstrate the comma operator evaluation order and full-expression lifetime of a temporary, like a pro
    -> hear all about commas and their equally evil cousins: the ternary operator and the go-to

    Fun fact: coincidentally, switching back to unique_lock but keeping the comma would signal a declaration problem, because a comma is not only an operator, but also a separator. C++ is Fun!

  16. The code at 15:16 is not broken at all, it works fine. I tried something similar under gcc 5.4 and checked it with ASAN and memcheck. Const l-value ref extend the lifetime of objects, and 2 slides later, when he binds it to auto& v, auto is deduced to const string and same thing happens.

  17. He made a mistake, you CANNOT create variable "var" of type "Type" by Type(var);
    But you can create ANONYMOUS variable by calling an appropriate constructor!
    If it has no name, it dies.
    Exactly the same thing happens when you call a non-void function but do not store its result.

  18. Did anybody else went and checked their "std::unique_lock lk(m_mutex)" code right after this?? That scared the crap out of me.

  19. I have one use of “volatile”, I stick it in my security code thats zeroing blocks of memory. I don’t want the compile nuking it, because thats a security risk.

  20. Doesn't seem applicable in 2019 $ g++ -Wall -o badref badref.cpp
    badref.cpp: In function ‘auto& get_default(std::__cxx11::string)’:
    badref.cpp:6:32: warning: reference to local variable ‘str’ returned [-Wreturn-local-addr]
    auto & get_default(std::string str)
    ^~~

  21. i almost never use index access. how come ? still it should be fairly easy to overload an operator and emit some kind of warning in case the index it too high .

  22. Louis, thank you for the great insights. I was wondering if ASAN is used at FB and if yes, what other sanitizers/tools have also a large impact on the security of FB's software stack. Thanks!

  23. That last bug with the mutex and shadowed lock (31:55). I had that before. Tore my hair out for a few days trying to figure out why my code wouldn't work.

Leave a Reply

Your email address will not be published. Required fields are marked *

Back To Top