StanfordPL / x64asm

x86-64 assembler library
Apache License 2.0
470 stars 60 forks source link

static map members of Label have undefined interaction with static data structures #168

Closed eschkufz closed 9 years ago

eschkufz commented 9 years ago

C doesn't guarantee an initialization order on static data structures. This causes Label's static maps to behave strangely when used as part of other static initialization code.

Specifically, see trying to create Code in a default constructor that is used as part of a cpputil arg.

eschkufz commented 9 years ago

Solve this by wrapping static members in static members and deferring initialization to first function call. Singleton. Classic.

bchurchill commented 9 years ago

At some point could you explain why the behavior was undefined to me? This seems like one of those C++ things I'm not familiar with.

On 02/26/2015 11:10 PM, eric schkufza wrote:

Closed #168 https://github.com/eschkufz/x64asm/issues/168 via e287b5f https://github.com/eschkufz/x64asm/commit/e287b5fffc1886c2219f2c7aba97e779cf82d91e.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#event-243092766.

eschkufz commented 9 years ago

C doesn't guarantee an initialization order among static variables in different translation units. So this code is undefined --

// a.h class X { static std::map<int,int> m; // m is defined }

// a.cc std::map<int,int> X::m // m is declared

// b.cc

include "a.h"

int global = X::m[0] // m is dereferenced

You can't guarantee that m has been declared / initialized when static storage and initial values for global are computed.

The reason this was a bug was because the cpputils arg library does a ton of stuff with static variables and it was having a strange interaction with the static maps inside the label class.

The solution is something like this --

// a.h class X { static std::map<int,int>& m() { static std::map<int,int> m; // m is defined and declared return m; } }

// a.cc // nothing here now

// b.cc

include "a.h"

int global = X::m()[0] // m is dereferenced

The semantics are identical, but the initialization code that the compiler emits is forced into a function which imposes an ordering on the evaluation of the dereference in the assignment to global. The map has to be initialized before the index takes place.

On Thu, Feb 26, 2015 at 11:13 PM, Berkeley Churchill < notifications@github.com> wrote:

At some point could you explain why the behavior was undefined to me? This seems like one of those C++ things I'm not familiar with.

On 02/26/2015 11:10 PM, eric schkufza wrote:

Closed #168 https://github.com/eschkufz/x64asm/issues/168 via e287b5f < https://github.com/eschkufz/x64asm/commit/e287b5fffc1886c2219f2c7aba97e779cf82d91e .

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#event-243092766.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349159.

eschkufz commented 9 years ago

This is the singleton design pattern.

You can take a look at cpputil::Singleton for an example. It's basically a templated version of this concept.

On Thu, Feb 26, 2015 at 11:21 PM, eric schkufza eric.schkufza@gmail.com wrote:

C doesn't guarantee an initialization order among static variables in different translation units. So this code is undefined --

// a.h class X { static std::map<int,int> m; // m is defined }

// a.cc std::map<int,int> X::m // m is declared

// b.cc

include "a.h"

int global = X::m[0] // m is dereferenced

You can't guarantee that m has been declared / initialized when static storage and initial values for global are computed.

The reason this was a bug was because the cpputils arg library does a ton of stuff with static variables and it was having a strange interaction with the static maps inside the label class.

The solution is something like this --

// a.h class X { static std::map<int,int>& m() { static std::map<int,int> m; // m is defined and declared return m; } }

// a.cc // nothing here now

// b.cc

include "a.h"

int global = X::m()[0] // m is dereferenced

The semantics are identical, but the initialization code that the compiler emits is forced into a function which imposes an ordering on the evaluation of the dereference in the assignment to global. The map has to be initialized before the index takes place.

On Thu, Feb 26, 2015 at 11:13 PM, Berkeley Churchill < notifications@github.com> wrote:

At some point could you explain why the behavior was undefined to me? This seems like one of those C++ things I'm not familiar with.

On 02/26/2015 11:10 PM, eric schkufza wrote:

Closed #168 https://github.com/eschkufz/x64asm/issues/168 via e287b5f < https://github.com/eschkufz/x64asm/commit/e287b5fffc1886c2219f2c7aba97e779cf82d91e .

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#event-243092766.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349159.

bchurchill commented 9 years ago

Cool, thanks!

Berkeley

On 02/26/2015 11:21 PM, eric schkufza wrote:

C doesn't guarantee an initialization order among static variables in different translation units. So this code is undefined --

// a.h class X { static std::map<int,int> m; // m is defined }

// a.cc std::map<int,int> X::m // m is declared

// b.cc

include "a.h"

int global = X::m[0] // m is dereferenced

You can't guarantee that m has been declared / initialized when static storage and initial values for global are computed.

The reason this was a bug was because the cpputils arg library does a ton of stuff with static variables and it was having a strange interaction with the static maps inside the label class.

The solution is something like this --

// a.h class X { static std::map<int,int>& m() { static std::map<int,int> m; // m is defined and declared return m; } }

// a.cc // nothing here now

// b.cc

include "a.h"

int global = X::m()[0] // m is dereferenced

The semantics are identical, but the initialization code that the compiler emits is forced into a function which imposes an ordering on the evaluation of the dereference in the assignment to global. The map has to be initialized before the index takes place.

On Thu, Feb 26, 2015 at 11:13 PM, Berkeley Churchill < notifications@github.com> wrote:

At some point could you explain why the behavior was undefined to me? This seems like one of those C++ things I'm not familiar with.

On 02/26/2015 11:10 PM, eric schkufza wrote:

Closed #168 https://github.com/eschkufz/x64asm/issues/168 via e287b5f <

https://github.com/eschkufz/x64asm/commit/e287b5fffc1886c2219f2c7aba97e779cf82d91e

.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#event-243092766.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349159.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349765.

eschkufz commented 9 years ago

Manolis could probably talk your ear off about this all day.

On Thu, Feb 26, 2015 at 11:23 PM, Berkeley Churchill < notifications@github.com> wrote:

Cool, thanks!

Berkeley

On 02/26/2015 11:21 PM, eric schkufza wrote:

C doesn't guarantee an initialization order among static variables in different translation units. So this code is undefined --

// a.h class X { static std::map<int,int> m; // m is defined }

// a.cc std::map<int,int> X::m // m is declared

// b.cc

include "a.h"

int global = X::m[0] // m is dereferenced

You can't guarantee that m has been declared / initialized when static storage and initial values for global are computed.

The reason this was a bug was because the cpputils arg library does a ton of stuff with static variables and it was having a strange interaction with the static maps inside the label class.

The solution is something like this --

// a.h class X { static std::map<int,int>& m() { static std::map<int,int> m; // m is defined and declared return m; } }

// a.cc // nothing here now

// b.cc

include "a.h"

int global = X::m()[0] // m is dereferenced

The semantics are identical, but the initialization code that the compiler emits is forced into a function which imposes an ordering on the evaluation of the dereference in the assignment to global. The map has to be initialized before the index takes place.

On Thu, Feb 26, 2015 at 11:13 PM, Berkeley Churchill < notifications@github.com> wrote:

At some point could you explain why the behavior was undefined to me? This seems like one of those C++ things I'm not familiar with.

On 02/26/2015 11:10 PM, eric schkufza wrote:

Closed #168 https://github.com/eschkufz/x64asm/issues/168 via e287b5f <

https://github.com/eschkufz/x64asm/commit/e287b5fffc1886c2219f2c7aba97e779cf82d91e

.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#event-243092766.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349159.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349765.

— Reply to this email directly or view it on GitHub https://github.com/eschkufz/x64asm/issues/168#issuecomment-76349902.