constexpr ctors and global initializers

[Primiano Tucci]

At the risk of stating the obvious, TIL that constexpr constructors are not sufficient to avoid static initializers. The fact that a constructor is trivial and marked as constexpr doesn't seem to be enough to guarantee that no code will be emitted for that ctor, which becomes quite relevant in the context of an object with static storage.

Hopefully this was just me over-engineering, but since this costed me 1.5 days of debugging on three different platforms, I thought it was worth sharing some details on this list to avoid frustration to the next one trying to do something similar.

Here is what I was trying to do:

I have a struct of pods: struct Foo {int x; int y; };

I want to have a non-const pre-populated global array of Foo. In other words I want some of the first elements to be initialized to non-zero values.

I want all this to happen without initializers. As we all know, initializers (!= linker initialized data) are evil because of unpredictable ordering of execution and annex slowdowns.

So, what I wanted to achieve here was "simply" induce the compiler to put my array in the .rwdata section (and whatever windows equivalent) without generating any code.

The easiest and most obvious way to achieve this is:

struct Foo {int x; int y}

Foo g_array[100] = { {1,1}, {2,2,} /*the other 98 elements will be zero initialized*/ }

AFAICT, this still works.

Here is what I got wrong:

I didn't want |x| and |y| to be public. I meant to give out pointers to elements of the array and I didn't want clients to be able to fiddle with the raw fields.

In other words I wanted |x| and |y| to be private and I wanted to create some public accessors for them. For the sake of this toy example let's say that I wanted |x|,|y| to be increment-only.

I thought it was reasonable to do something like this and expect that it would behave exactly as above:

class Foo {

public:

  constexpr Foo(int x, int y) x_(x), y_(y) {}

  constexpr Foo() x_(0), y_(0) {}

  int x() const { return x_; }

  void IncrementX() { ++x_; }

private:

  int x_; int y_;

  // DISALLOW_COPY_AND_ASSIGN if you want but doesn't seem to make a difference in all this.

}

Foo g_array[100] = { {1,1}, {2,2,} }

It doesn't. This seems to hit all sort of odd behaviors and create initializers for Foo.

FTR the specific CL where this happened is https://codereview.chromium.org/2452063003/ and the combination of bugs I hit is  crbug.com/660967 and crbug.com/660828 (constexpr ctors still creating static initializers under certain circumstances) which became an explosive combination when mixed with crbug.com/660376 (some test code having static initializers that use the code I was touching)

What does the style guide say?

At some point somebody in the bugs mentioned our code style. I gave it a re-read after the facts.

"Objects with static storage duration, including global variables, static variables, static class member variables, and function static variables, must be Plain Old Data (POD): only ints, chars, floats, or pointers, or arrays/structs of POD."

Foo in this case was a struct of POD, although with constexpr ctors. I mistakenly assumed this was still ok, it is not.

"The order in which class constructors and initializers for static variables are called is only partially specified in C++ and can even change from build to build, which can cause bugs that are difficult to find. Therefore in addition to banning globals of class type, we do not allow namespace-scope static variables to be initialized with the result of a function, unless that function (such as getenv(), or getpid()) does not itself depend on any other globals."

I think they key part that hit me is: "is a struct with a constrexpr ctor a class type?". 

I genuinely thought: "it's still a struct of pods". It is not.

"Variables of class type with static storage duration are forbidden: they cause hard-to-find bugs due to indeterminate order of construction and destruction. However, such variables are allowed if they are constexpr: they have no dynamic initialization or destruction."

Note, what this is saying is that if the entire array was constexpr, i.e. constexpr Foo g_array[100] = ... this would have been fine. But this is a different thing and is not a rw array anymore.

[Greg Thompson]

Thanks for the writeup. For the future, if you want to be sure that a type is POD, sprinkle a compile assert using std::is_pod (from <type_traits>) in a good place. I just learned about this, and am quite happy I did.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Primiano Tucci]

On Tue, Nov 1, 2016 at 11:53 AM Greg Thompson <g...@chromium.org> wrote:

Thanks for the writeup. For the future, if you want to be sure that a type is POD, sprinkle a compile assert using std::is_pod (from <type_traits>) in a good place. I just learned about this, and am quite happy I did.

Interesting. I was looking for a way to static_assert(does_not_cause_initializers<T>, ...) but didn't find anything. I didn't think to std::is_pod honestly. I just tried and effectively seems to work with the examples cited above.

[Peter Kasting]

On Tue, Nov 1, 2016 at 2:39 AM, Primiano Tucci <prim...@chromium.org> wrote:

I think they key part that hit me is: "is a struct with a constrexpr ctor a class type?". 

I genuinely thought: "it's still a struct of pods". It is not.

Right -- while declaring a constructor constexpr makes it available for compile-time calculations, it does not "take it out of the picture" when considering whether the struct qualifies as POD.  So such structs are non-POD.

"Variables of class type with static storage duration are forbidden: they cause hard-to-find bugs due to indeterminate order of construction and destruction. However, such variables are allowed if they are constexpr: they have no dynamic initialization or destruction."

Thanks for this quote!  This actually came up in a code review yesterday and I looked through the styleguide for something like this and couldn't find it, so ruled that "static constexpr MyClass = ..." was still invalid.  I was wrong :)

(Also thanks to Greg for the mention of std::is_pod -- type_traits is super helpful!)

PK

[Dmitry Skiba]

So from the first bug (crbug.com/660967) it seems that the problem with constexpr global objects is that MSVC handles them differently, and uses dynamic initialization instead of static initialization (as Clang / GCC)? This looks like MSVC bug (see also SO question, which quotes the standard).

I.e. once MSVC fixes that, we could allow global objects initialized by constexpr ctors?

  

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

[Peter Kasting]

On Tue, Nov 1, 2016 at 12:09 PM, Dmitry Skiba <dsk...@google.com> wrote:

So from the first bug (crbug.com/660967) it seems that the problem with constexpr global objects is that MSVC handles them differently, and uses dynamic initialization instead of static initialization (as Clang / GCC)? This looks like MSVC bug (see also SO question, which quotes the standard).

I.e. once MSVC fixes that, we could allow global objects initialized by constexpr ctors?

Our own Bruce Dawson replied on that SO question saying that MSVS 2015 Update 3 fixes this bug.

PK

[Antoine Labour]

On Tue, Nov 1, 2016 at 5:10 AM, Primiano Tucci <prim...@chromium.org> wrote:

On Tue, Nov 1, 2016 at 11:53 AM Greg Thompson <g...@chromium.org> wrote:

Thanks for the writeup. For the future, if you want to be sure that a type is POD, sprinkle a compile assert using std::is_pod (from <type_traits>) in a good place. I just learned about this, and am quite happy I did.

Interesting. I was looking for a way to static_assert(does_not_cause_initializers<T>, ...) but didn't find anything. I didn't think to std::is_pod honestly. I just tried and effectively seems to work with the examples cited above.

Note that what matters for std::is_pod is the default constructor and the copy constructor, which must be trivial (i.e. either  = default or omitted). It's still ok to have an explicit constexpr constructor, it just can't be the default one (or the copy constructor).

Antoine