[C++][Cppcon 2017] Louis Brandy: Curiously Recurring C++ Bugs at Facebook

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

Some wisdom extracted from writing production code..

ASan:
https://github.com/google/sanitizers


Bug 1:
std::vector::operator[]

1. No boundary check is performed (FAST)
2. Returns a reference

code:
--
return v[102];  // out of boundary.
--

mitigation:

• Static analysis. Complicated, Hard, Expensive.
• Improved abstractions (range-based operations)
• Dynamic analysis
• Bounds checking
• Address sanitizers (with 'fuzzing')
  https://fuzzing-project.org/tutorial2.html
• -fsanitize=address

Bug 2:
std::map::operator[] 

If you are a fan of Scott Meyers and have read his book:
Effective STL (Too bad there's no update with modern C++ since now he's retired.) Bug 1 & 2 are mentioned in this book.

1. It's slow.
2. Why it's slow? It checks if the key exist, if not, insert the key with
default value type constructor constructed value instance.

But, even though it's slow, it makes the code elegant , somehow~:
--
map<char, int> occ;

for (auto c : str) {
    occ[c]++;
}
--

const correctness:
As a senior C++ engineer knows const can act as guardian against accidentally modify the internal state.


Bug 3:
folly::get_default

Same idea as Python's  dict().get(Key, DefaultValue)

Bugs comes from returning string as r-value and caller references it.
(emm~ yup, this talk is for junior C++ engineer :-) although not bad and pinpoint the issue instead of gibberish )

mitigation:
-fsanitize-address-use-after-scope


Bug 4:
volatile

When coming to atomic operation, find something under
header <atomic>

The famous paper about volatile + atomic could be:
C++ and the Perils of Double-Checked Locking -Scott Meyers and Andrei Alexandrescu


Bug 5:
std::shared_ptr  thread safe?

Yes for internal ref counting.
(Another saying is that shared_ptr is as thead-safe as a normal pointer~ duh~ Not precise and sometimes incorrect.)

However, the type instance it points to is definitely not thread-safe.
(Well, depends on the type).

i.e shared_ptr's control block itself is thread-safe.
Wanna speed? Use unique_ptr.

Reference:
Always consider function thread safeness drags performance in single thread code

Reference:
$ git bisect  // debugging~
http://vsdmars.blogspot.com/2016/01/git-concepts-and-commands.html#more

mitigation:

• Thread sanitizer
• Address Sanitizer often does, too
• Use library:
  atomic<shared_ptr>

Bug 6:
This compiles:

#include <string>
void f()
{
    std::string(foo);  // Same as std::string foo;
}

Why?
Effective STL Item 6: Be alert for C++'s most vexing parse.

The insidious bug could be:

void obj::update() noexcept {
    unique_lock<mutex>(m_mutex);  // This one is truly insidious... Not locking at all.
    do_something();
}

Fix, simple name the unique_lock<mutex> g{m_mutext};
and use {} instead () _always_!

This kind of bug happens on types which have default constructor.

mitigation:

• -Wshadow  // Why? Because it warns us if there's a re-declaration shadows outter scope variable.
  It's noisy, not a good mitigation, though.
• Use {} instead of ()