Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

c++11: constant emitter needs to zero-initialize padding for trivially-copyable types #12114

Open
zygoloid mannequin opened this issue Jan 11, 2012 · 7 comments
Open
Labels
bugzilla Issues migrated from bugzilla c++11

Comments

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jan 11, 2012

Bugzilla Link 11742
Version unspecified
OS Linux
CC @DougGregor

Extended Description

Padding for objects with static storage duration is currently emitted as undef. This is non-conforming in C++11, because:

  1. zero initialization is performed prior to constant initialization (unlike C)
  2. zero initialization initializes padding to 0 bits (unlike C++98)
  3. for trivially-copyable types, it is permissible to inspect the effective array of unsigned char (of size sizeof(T)) comprising the object

Hence a conforming program can tell whether the padding has been zero-initialized.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2012

A testcase or three would be nice.

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented Nov 15, 2012

Simple example:

struct S { char c = 1; attribute((aligned(8))) char d = 2; } s = S();
char k = ((char*)&s)[1] + 1;

... gives ...

@​s = global %struct.S { i8 1, [7 x i8] undef, i8 2, [7 x i8] undef }, align 8

... which is wrong. The padding should be zeroed. Running this through opt -instcombine -globalopt gives:

@​k = global i8 undef, align 1

... which is wrong. k should be 1.

(Running this through just opt -globalopt strangely gives

@​k = global i8 1, align 1

which Nick tells me is a bug in globalopt.)

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 15, 2012

If the C++11 standard really says that padding must be initialized to zero
(your point 2) then indeed using undef is simply wrong.

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented Dec 3, 2012

If the C++11 standard really says that padding must be initialized to zero
(your point 2) then indeed using undef is simply wrong.

It really does. C++11 [dcl.init]p6:

"To zero-initialize an object or reference of type T means:
— if T is a scalar type (3.9), the object is set to the value 0 (zero), taken as an integral constant expression, converted to T;
— if T is a (possibly cv-qualified) non-union class type, each non-static data member and each base-class subobject is zero-initialized and padding is initialized to zero bits;
— if T is a (possibly cv-qualified) union type, the object’s first non-static named data member is zero-initialized and padding is initialized to zero bits;
— if T is an array type, each element is zero-initialized;
— if T is a reference type, no initialization is performed."

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 8, 2012

I partially fixed this in dragonegg in commit 169678. There remains the case
of something like this
A : char;
B : int;
which has implicit padding between A and B due to B's natural alignment. Must
this kind of padding also be zero initialized? Note that this is different to
your test case in a sense, though not so much in spirit.

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented Dec 9, 2012

I'm not sure what your notation means, but if you mean something like 'struct S { char A; int B; };', then yes, if an object of type S is zero-initialized, the padding between fields must be initialized to zero bits.

In Clang at least, such a struct is emitted as 'global %struct.S { i8 1, i32 2 }'. It's not clear to me whether the padding is zero or undef in this case.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 9, 2012

I'm pretty sure the padding is "undef" in this case.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla c++11
Projects
None yet
Development

No branches or pull requests

1 participant