0

Here is the example:

Main.cpp:

#include "MooFoobar.h"
#include "MooTestFoobar.h"

#include "FoobarUser.h"

namespace moo::test::xxx {
    struct X
    {
        void* operator new(const size_t size);

        FoobarUser m_User;
    };

    void* X::operator new(const size_t size)
    {
        printf("Allocated size: %zd\n", size);
        return malloc(size);
    }
} // namespace moo::test::xxx

int main()
{
    new moo::test::xxx::X;
    printf("Actual size: %zd, member size: %zd\n", sizeof(moo::test::xxx::X), sizeof(moo::test::xxx::FoobarUser));
    return 0;
}

MooFoobar.h:

namespace moo {
    struct Foobar
    {
        char m_Foo[64];
    };
} // namespace moo

MooTestFoobar.h:

namespace moo::test {
    struct Foobar
    {
        char m_Foo[32];
    };
} // namespace moo::test

FoobarUser.h:

#include "MooFoobar.h"

namespace moo::test::xxx {
    struct FoobarUser
    {
        FoobarUser();
        ~FoobarUser();

        Foobar m_Foobar;
    };
} // namespace moo::test::xxx

FoobarUser.cpp:

#include "FoobarUser.h"
#include <cstdio>

moo::test::xxx::FoobarUser::FoobarUser()
    : m_Foobar()
{
    printf("FoobarUser constructor, size: %zd\n", sizeof(*this));
}

moo::test::xxx::FoobarUser::~FoobarUser()
{}

So what is going on here: depending on order of includes unqualified name is resolved in different types and in FoobarUser.cpp we get size 64, in Main.cpp we get size 32. Not only sizeof is different - operator new is called with incorrect (32) size, but constructor will initialize size of 64, those leading to memory corruption.

In both clang and msvc the result of this program is:

Allocated size: 32
FoobarUser constructor, size: 64
Actual size: 32, member size: 32

This sounds very fishy and basically means that unqualified names are no-go if there is name-clash, because depending on include order it may lead to what essentially is incorrect program.

But I can't find any point in the C++ std that would say any of that invalid/ill-formed code. Can anyone help me?

Is it REALLY by standard and not some elaborate mass-compiler issue (though I can't really see how compilers can resolve that situation)?

YSC
  • 38,212
  • 9
  • 96
  • 149
Andrian Nord
  • 677
  • 4
  • 11
  • 2
    I don't get why it is surprising for you. When you invoke your compiler to compile `Main.cpp`, through inclusions it gets a definition of `Foobar` which differs from the one seen by `FoobarUser.cpp` when you invoke your compiler to compile that last translation unit. And yes, this is obviously a problem. – YSC Mar 07 '19 at 16:08
  • I meant "find through unqualified lookup" by "seen" or "gets". My bad. – YSC Mar 07 '19 at 16:12
  • Yes, I understand that it's "logical" thing to happen, except that it produces incorrect code and there is no warning or diagnostic that will say you "size is different, you are going to shoot yourself in a leg". Plus std usually says something like "if you do X program is ill-formed, no diagnostic required" – Andrian Nord Mar 07 '19 at 16:15
  • 2
    @AndrianNord ODR violation is not quite verifiable by implementations. It would require really expensive and complicated program to detect *most* of them, yet not all – Guillaume Racicot Mar 07 '19 at 16:17

1 Answers1

4

To answer rigorously to your question

I can't find any point in the C++ std that would say any of that invalid/ill-formed code. Can anyone help me?

This is [basic.def.odr]/12.2

There can be more than one definition of a class type [...] in a program provided that each definition appears in a different translation unit, and provided the definitions satisfy the following requirements. 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
  • in each definition of D, corresponding names, looked up according to [basic.lookup], shall refer to an entity defined within the definition of D, or shall refer to the same entity, after overload resolution and after matching of partial template specialization ([temp.over]), except that a name can refer to

    • [ ... nothing of relevance]

In your program, FoobarUser is defined in both of your translation units, but the name Foobar within refers to --- according to the rule of unqualified lookup --- two different entities (moo::test::FooBar and moo:FooBar). And this violates the One-Definition Rule. No diagnostic required.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • Yeah, I had suspiction it's ODR, but I was expecting "this is ODR-violation". It didn't appear to me to look for odr definition. Thank you – Andrian Nord Mar 07 '19 at 16:25