23

I had this conversation with a colleague, and it turned out to be interesting. Say we have the following POD class

struct A { 
  void clear() { memset(this, 0, sizeof(A)); } 

  int age; 
  char type; 
};

clear is intended to clear all members, setting to 0 (byte wise). What could go wrong if we use A as a base class? There's a subtle source for bugs here.

Jonas
  • 121,568
  • 97
  • 310
  • 388
Johannes Schaub - litb
  • 496,577
  • 130
  • 894
  • 1,212

5 Answers5

18

The compiler is likely to add padding bytes to A. So sizeof(A) extends beyond char type (until the end of the padding). However in case of inheritance the compiler might not add the padded bytes. So the call to memset will overwrite part of the subclass.

StackedCrooked
  • 34,653
  • 44
  • 154
  • 278
  • @Luchian I don't understand your point? I think that `sizeof(int) + sizeof(char) = 5` which isn't aligned on a 32-bit architecture.. – StackedCrooked Aug 18 '11 at 20:13
  • @Luchian: What do you think `sizeof(A)` would be? The answer may surprise you. – Fred Larson Aug 18 '11 at 20:14
  • @Fred Larson sizeof(A) depends on sizeof(int) on his platform. And again, in this case, it's sizeof(int) + sizeof(char). If the member order was inverted, it would be sizeof(char) + int_padding - 1 + sizeof(int). – Luchian Grigore Aug 18 '11 at 20:16
  • @Luchian: Guess again. Try it. – Fred Larson Aug 18 '11 at 20:17
  • @Luchian: Yep, same results with VC10. sizeof(A) == 8, that covers 2 of the most common compilers out in use. – Benjamin Lindley Aug 18 '11 at 20:20
  • @Luchian padding is definitely added to the end of structures as well as between members. This is because there is nothing to prohibit storing the structures in, say, an array, where the beginning of each almost certainly needs to be aligned (assuming anything does). – Karl Knechtel Aug 18 '11 at 20:20
  • 2
    You're right, I thought padding was added only between members to keep alignment. Live and learn. Thanks gents! :) – Luchian Grigore Aug 18 '11 at 20:23
  • @Luchian Grigore The reason is simple: A[10] wouldn't be valid if we didn't add padding (ie the compiler is even forced to align the struct correctly, otherwise it violates the c standard!) – Voo Aug 18 '11 at 20:44
5

In addition to the other notes, sizeof is a compile-time operator, so clear() will not zero out any members added by derived classes (except as noted due to padding weirdness).

There's nothing really "subtle" about this; memset is a horrible thing to be using in C++. In the rare cases where you really can just fill memory with zeros and expect sane behaviour, and you really need to fill the memory with zeros, and zero-initializing everything via the initializer list the civilized way is somehow unacceptable, use std::fill instead.

Karl Knechtel
  • 62,466
  • 11
  • 102
  • 153
  • I don't see how `std::fill` would help in this particular case though - what iterator would you use? – Voo Aug 18 '11 at 20:49
  • `char* begin = reinterpret_cast(this); char* end = begin + sizeof(this); std::fill(begin, end, 0);` would be the idiomatic translation I guess, but it has all the same problems really. Honestly I would probably rather just make auto-zero-initializing wrappers for primitive types and compose structs from them instead. But the point is that `std::fill` is a more sophisticated tool for filling memory and costs us nothing, with a decent compiler, in the cases where plain old `memset` would work. – Karl Knechtel Aug 18 '11 at 20:52
  • 1
    @Karl : "*Honestly I would probably rather just make auto-zero-initializing wrappers for primitive types*" See [Boost.ValueInitialized](http://www.boost.org/doc/libs/release/libs/utility/value_init.htm). – ildjarn Aug 18 '11 at 20:57
  • 1
    @Karl Oh I agree that there's no reason to use memset or other old c functions in c++ code, but in this case fill and memset have pretty much the same problems and solutions (and I'd think the compiler would generate the same code anyhow). – Voo Aug 18 '11 at 21:14
2

In theory, the compiler can lay out base classes differently. C++03 §10 paragraph 5 says:

A base class subobject might have a layout (3.7) different from the layout of a most derived object of the same type.

As StackedCrooked mentioned, this might happen by the compiler adding padding to the end of the base class A when it exists as its own object, but the compiler might not add that padding when it's a base class. This would cause A::clear() to overwrite the first few bytes of the members of the subclass.

However in practice, I have not been able to get this to happen with either GCC or Visual Studio 2008. Using this test:

struct A
{
  void clear() { memset(this, 0, sizeof(A)); }

  int age;
  char type;
};

struct B : public A
{
  char x;
};

int main(void)
{
  B b;
  printf("%d %d %d\n", sizeof(A), sizeof(B), ((char*)&b.x - (char*)&b));
  b.x = 3;
  b.clear();
  printf("%d\n", b.x);

  return 0;
}

And modifying A, B, or both to be 'packed' (with #pragma pack in VS and __attribute__((packed)) in GCC), I couldn't get b.x to be overwritten in any case. Optimizations were enabled. The 3 values printed for the sizes/offsets were always 8/12/8, 8/9/8, or 5/6/5.

Community
  • 1
  • 1
Adam Rosenfield
  • 390,455
  • 97
  • 512
  • 589
  • 1
    Here is my testcase. The base is not a POD though: http://ideone.com/GHATf . Initially I didn't notice that GCC would behave different for PODs, so I worded the question in a way that makes the issue stand out stronger. But as you say, the possibility still exists. And I bet many people will think that a silly `private` there has no effect. Also worth noting 3.9p3 and 3.9p2 where it contains the remarks "... where neither obj1 nor obj2 is a base-class subobject ..." to support the different layouting. – Johannes Schaub - litb Aug 18 '11 at 21:35
0

Briefly: It seems to me that the only one potentional problem is in that I can not found any info about "padding bytes" guarantees in C89, C2003 standarts....Do they have some extraordinary volatile or readonly behavior - I can not find even what does term "padding bytes" mean by the standarts...

Detailed:

For objects of POD types it is guaranteed by the C++2003 standard that:

  • when you memcpy the contents of your object into an array of char or unsigned char, and then memcpy the contents back into your object, the object will hold its original value

  • guaranteed that there will be no padding in the beginning of a POD object

  • can break C++ rules about: goto statement, lifetime

For C89 there is also exist some guarantees about structures:

  • When used for a mixture of union structures if structs have same begining, then first compoments have perfect mathing

  • sizeof structures in C is equal to the amount of memory to store all the components, the place under the padding between the components, place padding under the following structures

  • In C components of the structure are given addresses. There is a guarantee that the components of the address are in ascending order. And the address of the first component coincides with the start address of the structure. Regardless of which endian the computer where the program runs

So It seems to me that such rules is appropriate to C++ also, and all is fine. I really think that in hardware level nobody will restrict you from write in padding bytes for non-const object.

Konstantin Burlachenko
  • 5,233
  • 2
  • 41
  • 40
0

The clear method of the base class will only set the values of the class members.

According to alignment rules, the compiler is allowed to insert padding so that the next data member will occur on the aligned boundary. Thus there will be padding after the type data member. The first data member of the descendant will occupy this slot and be free from the effects of memset, since the sizeof the base class does not include the size of the descendant. Size of parent != size of child (unless child has no data members). See slicing.

Packing of structures is not a part of the language standard. Hopefully, with a good compiler, the size of a packed structure does not include any extra bytes after the last. Even so, a packed descendant inheriting from a packed parent should produce the same result: parent sets only the data members in the parent.

Thomas Matthews
  • 56,849
  • 17
  • 98
  • 154