A serious bug in GCC

This post is to inform you about a bug in GCC that may cause memory (or other resource) leaks in your valid C++ programs.

One of the pillars of C++ philosophy is what we often call RAII: if you use classes to manage resources, use constructors for allocating resources and destructors for releasing them, the language makes sure that whatever happens, however you use such classes the resources will get properly released. We can easily test this guarantee by a fake class that logs resource acquisitions and releases:

struct Resource
{
  explicit Resource(int) { std::puts("create"); }
  Resource(Resource const&) { std::puts("create"); }
  ~Resource() { std::puts("destroy"); }
};

Whatever reasonable program we write (excluding the situations where you use raise, longjmp, exit, abort, etc., or when we cause std::terminate to be called) we expect that "create" and "destroy" are output the same number of times.

This is the contract: I take care that my classes correctly manage resources, and the language takes care that the resources will always be managed correctly regardless of the complexity of the program. This even works for such complex situations, you might not even have thought of:

Resource make_1() { return Resource(1); }
Resource make_2() { throw std::runtime_error("failed"); }

class User
{
  Resource r1;
  Resource r2;

public:
  explicit User()
    : r1{make_1()}
    , r2{make_2()} // what if make_2() throws?
    {}
};

Consider what happens when make_2() throws when executing this constructor. r1 has already been constructed (resources acquired), but object User has not been created yet, and it will never be (because constructor will not run to a successful end). This means that destructor of User will never be called either. But the language is still required to call the destructor of any sub-object that has been successfully created, like r1. Thus, r1’s resources will nonetheless be released, even though no object of type User was ever fully constructed.

You might have not even heard about this guarantee, but it still works to your advantage, preventing memory leaks.

But in one situation GCC will surprise you: namely, when you initialize a temporary using aggregate initialization. Let’s change our type User a bit, so that it is an aggregate:

struct User 
{
  Resource r1;
  Resource r2;
};

It just aggregates members. No constructors, but we can still initialize it with aggregate initialization syntax:

void process (User) {}

int main()
{
  try { 
    User u {make_1(), make_2()};
    process(u);
  }
  catch (...) {}
}

If you test it, it works correctly: the number of constructor calls equals the number of destructor calls, even though make_2() throws and makes the situation complicated. But u is an automatic object. If we change the example, and create a temporary User instead:

int main()
{
  try { 
    process({make_1(), make_2()});
  }
  catch (...) {}
}

This is where the bug manifests. Member r1 is initialized but never destroyed. Admittedly, this is a rare case: it requires an exception in the middle of initialization, a temporary and an aggregate initialization. But usually, leaks manifest in the face of exceptions. And the fact that it is rare makes you less prepared for it.

Here is a full example:

#include <cstdio>
#include <stdexcept>

struct Resource
{
  explicit Resource(int) { std::puts("create"); }
  Resource(Resource const&) { std::puts("create"); }
  ~Resource() { std::puts("destroy"); }
};

Resource make_1() { return Resource(1); }
Resource make_2() { throw std::runtime_error("failed"); }

struct User 
{
  Resource r1;
  Resource r2;
};

void process (User) {}

int main()
{
  try {
    process({make_1(), make_2()});
  }
  catch (...) {}
}

You can test it online here. It is present in GCC 4, 5, and 6. For a more real-life, and somewhat longer, illustration of the problem, see this example provided by Tomasz Kamiński.

A bug report for this already exists.

Maybe your program already leaks because of this surprise?

Advertisements
This entry was posted in programming and tagged , . Bookmark the permalink.

27 Responses to A serious bug in GCC

  1. Thanks for sharing! Just a nit: <cstdio> is not guaranteed to provide puts in the global namespace.

  2. John says:

    Lmao don’t do this then. I love C++ and I follow RAII.

  3. Jinank Jain says:

    Good part is I tried out with other compilers like clang it was working as expected

  4. tomaszkam says:

    This bug causes even code using initializer list to leak, for example:
    std::vector vs{s1, s2, s3};
    Will leak if construction of s2 or s3 will fail. Live example: https://wandbox.org/permlink/VZiD1OtYgggZuLwk.

  5. Jesse Maurais says:

    A little correction here. The r1 member is not necessarily initialized because the order of construction is not guaranteed. I’d have to double check the standard to be sure, but I believe the order is an implementation detail left the compiler’s discretion.

    • tomaszkam says:

      Elements in brace-initializer ({e1, e2, e3}) are required to be initialized from left-to-right by the standard. For reference: http://stackoverflow.com/questions/14060264/order-of-evaluation-of-elements-in-list-initialization

      • Jesse Maurais says:

        Sorry, I wasn’t referring to the brace initialization. Let me clarify

        class User
        {
        Resource r1;
        Resource r2;

        public:
        explicit User()
        : r1{make_1()}
        , r2{make_2()} // what if make_2() throws?
        {}
        };

        In the User() constructor there is no guarantee that r1 is initialized before r2.

        • Anonymous Coward says:

          The standard guarantees initialization order is the declaration order of members (regardless of the order in the initializer list).

        • Chris says:

          Actually, the standard draft (n4296 §12.6.2/13.3) defines that they are constructed in the order they are declared in the class.

          So it does not matter that “r1{make_1()}” is before “r2{make_2()}”, but because “Resource r1;” is before “Resource r2;”, r1 will be initialized first.

        • nobody says:

          You are wrong. Standard says class members are constructed in order they are defined. So r1 before r2.

        • Dinka says:

          Hello,
          In this case, r1 is guaranteed to be initialised before r2.

          from N4618 : [class.base.init]/p13
          “In a non-delegating constructor, initialization proceeds in the following order:

          — Then, non-static data members are initialized in the order they were declared in the class definition (again regardless of the order of the mem-initializers).”

          Best,
          Dinka

    • dv says:

      Of course, you are wrong. If you were right, and initialisation order was not absolutely guaranteed on a basic level, it would make classes in C++ a joke and many programs undefined and impossible. Thankfully, they knew this and competently designed this basic area.

  6. Pingback: Dev Digest Episode 93

  7. Pingback: BYB 1×10 – Terratenientes, ñues y campamentos | Birras y Bits

  8. Pingback: C++ Annotated: Apr – Aug 2017 - ReSharper C++ BlogReSharper C++ Blog

  9. Pingback: C++ Annotated: Apr – Aug 2017 | CLion Blog

  10. Chris says:

    Thanks for this Post.
    Do you know scott meyer’s book?
    Whenever a exception raise inside ctor or dtor, then your object is in undefine state.

    Cheers
    Chris

    • Actually, this statement is not correct. From the place where the exception was thrown you can determine which of your object’s sub-objects are fully constructed and fully destroyed. It would be unwise to manually call functions on your sub-objects, but the mechanism for automatically calling destructors of fully created sub-objects works fine. This is the case even for destructors. For more information, see this post.

  11. Bob says:

    I’ve seen something weird regarding default constructing unordered_map using brace initialization (as a function parameter default). Causes crash in unordered_map destructor, which looks like a double delete. Don’t think it’s related to this one though, just wondering if anyone else has had this?

    gcc version 4.4.7 20120313

    Cannot reproduce outside our code base. But basically:

    void somefunc(
    std::unordered_map map1 = {{}},
    std::unordered_map map2 = {{}}
    )

    Crashes in std::unordered_map destructor. When I change it to:

    void somefunc(
    std::unordered_map map1 = std::unordered_map(),
    std::unordered_map map2 =std::unordered_map()
    )

    works fine.

  12. Bob says:

    Got it:

    #include
    #include

    void TestTest
    (
    bool call2ndTime,
    std::unordered_map a = {{}},
    std::unordered_map b= {{}}
    )
    {
    if(call2ndTime)
    TestTest(false, std::move(a), std::move(b));
    }

    int main()
    {
    TestTest(true);
    }

    Hard to say exactly where it messes up.

    *** glibc detected *** ./myfile: double free or corruption (fasttop): 0x000000000258b050 ***

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s