1

I've encountered a segmentation fault in a C++ program when two C++ files compiled together each include a different definition of a structure (with the same name).

According to this question, I understand that structure definitions are restricted to the translation unit (the file and its inclusions).

However, I get a crash when enabling -O1 or more at compile time. The following minimal code reproduced the segfault.

The code is in 3 short C++ files and 2 headers:

// td_collision1.cc
#include <iostream>
#include <vector>
#include <cstdlib>
#include "td1.h"

struct Data
{
  long a;
  double m1;
  double m2;
};

void sz1(void) {
    std::cout << "Size of in collision1: " << sizeof(struct Data) << std::endl;
}

void collision1(void) {
    struct Data tmp;
    std::vector<struct Data> foo;
    for (int i=0; i<10; i++) {
        tmp.a = 1;
        tmp.m1 = 0;
        tmp.m2 = 0;
        foo.push_back(tmp);
    }
}
// td1.h
#include <iostream>

void collision1(void);
void sz1(void);

// td_collision2.cc
#include <iostream>
#include <vector>
#include <cstdlib>
#include "td2.h"

struct Data {
  long a;
  double m1; // note that there is one member less here
};

void sz2(void) {
    std::cout << "Size of in collision2: " << sizeof(struct Data) << std::endl;
}

void collision2(void) {
    struct Data tmp2;
    std::vector<struct Data> bar;
    for (int i=0; i<100; i++) {
        tmp2.a = 1;
        tmp2.m1 = 0;
        bar.push_back(tmp2); // errors occur here
    }
}
// td2.h
#include <iostream>

void collision2(void);
void sz2(void);

// td_main.cc
#include <iostream>
#include <cstdlib>
#include "td1.h"
#include "td2.h"

int main(void) {
    sz1();
    sz2();
    collision2();
}

This code compiled with GCC 6.3 with -O0 flag runs fine and without error under valgrind. However, running it with -O1 or O2 leads to the following output:

Size of in collision1: 24
Size of in collision2: 16
==326== Invalid write of size 8
==326==    at 0x400F6C: construct<Data, const Data&> (new_allocator.h:120)
==326==    by 0x400F6C: construct<Data, const Data&> (alloc_traits.h:455)
==326==    by 0x400F6C: push_back (stl_vector.h:918)
==326==    by 0x400F6C: collision2() (td_collision2.cc:22)
==326==    by 0x400FE8: main (td_main.cc:10)
==326==  Address 0x5aba1f0 is 0 bytes after a block of size 96 alloc'd
==326==    at 0x4C2E1FC: operator new(unsigned long) (vg_replace_malloc.c:334)
==326==    by 0x400DE9: allocate (new_allocator.h:104)
==326==    by 0x400DE9: allocate (alloc_traits.h:416)
==326==    by 0x400DE9: _M_allocate (stl_vector.h:170)
==326==    by 0x400DE9: void std::vector<Data, std::allocator<Data> >::_M_emplace_back_aux<Data const&>(Data const&) (vector.tcc:412)
==326==    by 0x400F7E: push_back (stl_vector.h:924)
==326==    by 0x400F7E: collision2() (td_collision2.cc:22)
==326==    by 0x400FE8: main (td_main.cc:10)
==326== 
==326== Invalid write of size 8
==326==    at 0x400F69: construct<Data, const Data&> (new_allocator.h:120)
==326==    by 0x400F69: construct<Data, const Data&> (alloc_traits.h:455)
==326==    by 0x400F69: push_back (stl_vector.h:918)
==326==    by 0x400F69: collision2() (td_collision2.cc:22)
==326==    by 0x400FE8: main (td_main.cc:10)
==326==  Address 0x5aba1f8 is 8 bytes after a block of size 96 alloc'd
==326==    at 0x4C2E1FC: operator new(unsigned long) (vg_replace_malloc.c:334)
==326==    by 0x400DE9: allocate (new_allocator.h:104)
==326==    by 0x400DE9: allocate (alloc_traits.h:416)
==326==    by 0x400DE9: _M_allocate (stl_vector.h:170)
==326==    by 0x400DE9: void std::vector<Data, std::allocator<Data> >::_M_emplace_back_aux<Data const&>(Data const&) (vector.tcc:412)
==326==    by 0x400F7E: push_back (stl_vector.h:924)
==326==    by 0x400F7E: collision2() (td_collision2.cc:22)
==326==    by 0x400FE8: main (td_main.cc:10)
==326== 
==326== 
==326== HEAP SUMMARY:
==326==     in use at exit: 0 bytes in 0 blocks
==326==   total heap usage: 5 allocs, 5 frees, 73,896 bytes allocated
==326== 
==326== All heap blocks were freed -- no leaks are possible
==326== 
==326== For counts of detected and suppressed errors, rerun with: -v
==326== ERROR SUMMARY: 191 errors from 2 contexts (suppressed: 0 from 0)

the push_back() function fails when the libc reallocates std::vector<struct Data> bar. (in my case, its size is 4 items initially, and the vector is further resized afterwards when calling push_back() in the loop.) When struct Data in td_collision1.cc has the same size as in td_collision2.cc, the program doesn't crash.

Therefore, there seem to be a collision between these two structure definitions. Indeed, if I rename one structure, the bug obviously vanishes. But, as mentioned above, I thought that this could not happen. What did I misunderstood? Also, if I get rid of function collision1(), the segfault vanishes (struct Data in collision1 is probably ditched by the compiler because unused)

My understanding was that there exist a clear separation between these two CC files and no "crosstalk" should be possible if the structures are not present in the header.

Edit: add missing td2.h

user7378105
  • 31
  • 3
  • 3
    It's undefined behavior – Danh Jan 05 '17 at 09:04
  • 3
    [*One Definition Rule*](http://en.cppreference.com/w/cpp/language/definition#One_Definition_Rule). "I understand that structure definitions are restricted to the translation unit (the file and its inclusions)." - Not 100% sure I understand what you mean, but that sounds wrong. If you want something that is local only to a particular `.cpp` file, put it in an [*unnamed namespace*](http://en.cppreference.com/w/cpp/language/namespace#Unnamed_namespaces). – BoBTFish Jan 05 '17 at 09:05
  • 3
    Seriously, Why tag C? You don't provide a [mcve], What is it in "td2.h"? I can't reproduce! I loose my time with your question. – Stargateur Jan 05 '17 at 09:07
  • *Off topic: you can use `foo.push_back(Data{1, 0, 0});` or since C++11 `foo.emplace_back(1, 0, 0);` to shorten a code* – Pavel Jan 05 '17 at 09:33
  • @Stargateur: you're right, it was incorrectly tagged (I wrongly followed the suggestions of the system). Also I missed one (straightforward) include. Right again. That said, nobody forces you to read and answer my questions. – user7378105 Jan 05 '17 at 12:39
  • @user7378105 Sorry, I was too aggressive. I'm not a native english speaker so I only do short sentence. That lead to be less verbose. – Stargateur Jan 05 '17 at 12:45
  • @BoBTFish that's what I was looking for I guess, thank you. – user7378105 Jan 05 '17 at 12:56

3 Answers3

7

The answer you linked is for C language, and C is not C++.

In C++ (quote from en.cppreference, see Danh's answer for the standard), the rule is as follow:

There can be more than one definition in a program, as long as each definition appears in a different translation unit, of each of the following: class type [...], as long as all of the following is true:

  • each definition consists of the same sequence of tokens (typically, appears in the same header file)

  • [...]

If all these requirements are satisfied, the program behaves as if there is only one definition in the entire program. Otherwise, the behavior is undefined.

Your two definitions clearly violates the first condition, so the behavior is undefined.

Community
  • 1
  • 1
Holt
  • 36,600
  • 7
  • 92
  • 139
  • "In C, you can have multiple definitions of the same type in different translation units" - That's wrong! The _one definition rule_ also applies to C, of course. – too honest for this site Jan 05 '17 at 12:16
  • @Olaf The *One Definition Rule* is a C++ concept, not applicable as it is to C. C does not define a *One Definition Rule*, the closest you can find is probably 6.27/3 (C11): *"All declarations that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined."*. Actually, you can have multiple definitions in C++, as long as the definitions follows the rule defined by [basic.def.odr](http://eel.is/c++draft/basic.def.odr#6), the C++ rules are simply stricter than the C ones. – Holt Jan 05 '17 at 12:25
  • Thank you. I am not a C++ expert as you can see. However I ran into that issue on a project and was very surprised by this behavior because: 1) the compiler doesn't warn me, 2) I expected a clear separation between the code in each file, since nothing is shared via headers. edit: I guess that's why namespaces should be used... – user7378105 Jan 05 '17 at 12:52
  • @Holt: Read the citation you posted in your comment **carefully** again: "All **declarations** that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined." - **Declarations**, not *definitions** as you write in your answer! Something like `int i;` at file-scope is a _tentative_ definition only. It becomes a definition if ther is no "true" definition (i.e. with initialiser), in which case all tentative definitions are fold into a single definition (resp. the first tentative definition becomes a full definition, the rest is treated as declaration. – too honest for this site Jan 05 '17 at 12:57
  • Strictly speaking there are no "type definitions". `typedef is an alias and _declarations_ is basically what the standard provides, more exactly: type specifiers as part of a _declaration_. But that is about variable declaration and definiton, not about types. You confuse very different concepts - not helpful! You want to search for "tentative definition" in the standard. – too honest for this site Jan 05 '17 at 13:35
  • @Olaf I have deleted my last comment and removed the part of the answer about C which was in all cases irrelevant here. Even after going through a huge part of the C standard, this particular case is still unclear for me, so I will dig a bit more before adding anything... If you have any strong references to the standard, I'll be happy to read them. – Holt Jan 05 '17 at 13:35
  • @Holt: I'll leave my comment nevertheless, as it might help you to get the pieces together by reading the standard with it in mind. Indeed, as you stated C and C++ are different languages and have diverged even more with C++11 and C99 (C11 mostly adds features, but is otherwise quite close to C99). – too honest for this site Jan 05 '17 at 13:38
1

From basic.def.odr, (... is omitted by me):

There can be more than one definition of a class type (Clause [class]), ..... Given such an entity named D defined in more than one translation unit, then:

  • each definition of D shall consist of the same sequence of tokens; and
  • ...

If D is a template and is defined in more than one translation unit, then the preceding requirements shall apply both to names from the template's enclosing scope used in the template definition ([temp.nondep]), and also to dependent names at the point of instantiation ([temp.dep]). If the definitions of D satisfy all these requirements, then the behavior is as if there were a single definition of D. If the definitions of D do not satisfy these requirements, then the behavior is undefined.

In your program, definition of struct Data in td_collision1.cc and in td_collision2.cc doesn't match with each other, hence, definitions of struct Data do not satisfy those requirements, then the behavior is undefined.

Danh
  • 5,916
  • 7
  • 30
  • 45
0

Well, you're linking a C answer, but your question is about C++. Two languages, two standards, two answers.

That said, I believe the C answer should also be that it's disallowed, per the One Definition Rule (which both languages have). Violating that is Undefined Behavior, which includes Segmentation Faults.

MSalters
  • 173,980
  • 10
  • 155
  • 350