Are static locals initialized safely?

[Pavel Kalinnikov]

Hi cxx wise folks,

In the list of C++11 features I couldn't find anything about static local variables. The new standard claims they should be thread-safely initialized once. Can we rely on this in chromium?

Thank you,

Pavel

[Vasilii Sukhanov]

It's still prohibited for Chromium (see the C++ Dos and Don'ts). Although the relevant thread is 5 years old.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CADF%2BPP7bQxVu_CnkYrM53FPqfWtNWnOCnzw93%3DNy_%3DisRDhmwQ%40mail.gmail.com.

[Mark Mentovai]

Normally they are available in standard C++11, but this is not true in Chromium.

https://chromium.googlesource.com/chromium/src/+/63384a79de37f89a88cfc02ff61eb649e4abe535/build/config/compiler/BUILD.gn#181

https://chromium.googlesource.com/chromium/src/+/7bff5aca5d8144576a46b3dc0a37086c7e1a8d25/build/config/win/BUILD.gn#59

Background: When Chromium was an infant, MSVC had no support for thread-safe statics. (This was pre-C++11, so you couldn’t really blame them for this omission.) We had to come up with other ways to initialize these variables safely, like LazyInstance and Singleton. When we started working on Mac and later Linux, we were using GCC, which did have thread-safe static initialization. But since all of our code already had to work around MSVC’s lack of support and deal with this itself, the compiler support in GCC was just dead weight, so we disabled it by specifying -fno-threadsafe-statics. Years later, MSVC grew support, but by then, we had many times more thread-safe statics, and who wanted to go back and remove them all? So we specified /Zc:threadSafeInit- there and retained the previous behavior.

Personally, I’d love to move toward the language standard, turn on compiler support for thread-safe static initialization, and allow it to be used anywhere sensible.

--

[Nico Weber]

Part if why we use /Zc:threadSafeInit- is because VS2015 generates a _lot_ if code for each static if you don't pass it, and binary size regressed quite a bit without it (if you blame the gn file passing that flag, you'll find a bug with numbers).

[Pavel Kalinnikov]

Thanks everyone for very elaborate responses!

[Scott Graham]

Hi, I plan to flip us to removing /Zc:threadSafeInit- and -fno-threadsafe-statics, i.e. making static initialization thread safe by default.

This is in service of a particular bug, but adding a bunch of workaround code for that problem at this point seems a bit silly.

If anyone objects, or knows of reasons why this won't work please let me know. I plan to proceed by first making our compile options threadsafe, and then recovering the code size penalty (approximately 54k binary size on Windows), as it of course can't be done in the other order. I believe quite a bit of the size will be recovered by the removal of code in LazyInstance instances, but I don't know if it will be an overall net win/loss.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CADF%2BPP4%2BLmVYBpKFz4WFTLWO1wKFhyV3h8kNGM2YptXUrQSCUA%40mail.gmail.com.

[Scott Graham]

(A bug for doing that: https://bugs.chromium.org/p/chromium/issues/detail?id=686866.)

[Dmitry Skiba]

So, the situation with GCC/clang is the same? I.e. they create dedicated 'state' static and write it before writing the data?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANHK6RZd%2ByOiGywOLj-EnwvCdTzMsSVu82OQs-WuUbrhMzAQAg%40mail.gmail.com.

[Mark Mentovai]

We have suppressed this feature of GCC and clang by specifying -fno-threadsafe-statics for compatibility with old MSVC, but yes, both GCC and clang have supported thread-safe static initialization for a very long time. Far longer than C++ has required it, in fact.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABWS%2BBx%3DpQrCi3xnF5pTD%3DeDLoy1YKQTaVEDDWAvHgdUxqW5ag%40mail.gmail.com.

[Nico Weber]

Sounds great. Thanks, Scott!

[Bruce]

Yep, sounds great. Go for it. I'm sure that most of the binary size regression can indeed be recovered, and what remains will be the reasonable price to pay for having local statics behave sanely.

[Dmitry Skiba]

Once this is done, will we update our coding guide? Relying on thread-safe statics can simplify things in some cases.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/b5e18d5b-711f-4e5f-bbbd-3cf7bfa3420e%40chromium.org.

[Scott Graham]

Yes, I'll update the style guide. The change to flip behaviour has landed, and I'm looking at simplifying some LazyInstance<T> usage now.

(If anyone feels enthused to simplify code in some subtree, please feel encouraged, and mention what you're attacking at crbug.com/686866.)

[Yuri Wiitala]

Can someone clarify: Is the goal to remove the use of LazyInstance where thread-safe static locals can now be used and would be simpler? Or, are we trying to remove *all* uses of LazyInstance everywhere? Or, something else?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANHK6RbkXWL93WDoJ9JKxL9S-t-6ro_r8UzudsTYroqZ8nKvMw%40mail.gmail.com.

[Scott Graham]

I believe we should:

- try to not use LazyInstance in new code;

- replace code that uses LazyInstance where it could be replaced by less code (this seems to apply especially to LazyInstance::Leaky);

- maintain LazyInstance in more complex cases where it adds some value.

Moving away from LazyInstance is not urgent, and should just be considered a low-priority clean up and simplification task, perhaps something to do when you notice it in a file you're working in. When doing so, I think it's also valuable to (re-)consider whether a non-leaky LazyInstance could really have been Leaky.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CA%2BN%2BEKZkW9h99ajKoEq3vThPpjr7pbgn65CqF8FgqpwQFmXYWg%40mail.gmail.com.

[Dale Curtis]

After converting all of the */media/ targets, I didn't find a single case where continuing to use LazyInstance made sense. They are all fairly easy conversions to the form of GetXXX() where that method has an internal static. The biggest win in our case was simplifying code of the pattern "initialize&&store bool" type classes into a internal function with a static bool.

https://codereview.chromium.org/2668813002 

- dale

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CANHK6RZh_xyC-PymG0bBmiCU1-h6s%2BLqs_au%3D4qkN%3DrYKT7vmw%40mail.gmail.com.

[Dmitry Skiba]

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

?

[dan...@chromium.org]

On Tue, Jan 31, 2017 at 3:57 PM, 'Dmitry Skiba' via cxx <c...@chromium.org> wrote:

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

?

AFAIK it does not say anything and doesn't give preference to returning pointers vs references.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABWS%2BBy%2BfzXWJQc7KHzUF0rodm3ch1BiRN9HdjTWkosuYBn7CA%40mail.gmail.com.

[Peter Kasting]

On Tue, Jan 31, 2017 at 12:57 PM, 'Dmitry Skiba' via cxx <c...@chromium.org> wrote:

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

From http://google.github.io/styleguide/cppguide.html#Reference_Arguments :

"it is a very strong convention in Google code that input arguments are values or const references while output arguments are pointers"

The rest of that section is slightly ambiguous, because it speaks strongly of disallowing non-const refs as parameters, but it mostly sounds like it's speaking about input parameters, except for this sentence that speaks about "output arguments".

However, at the least, I think this suggests preferring return-by-pointer, even if it's not mandated.  In most non-Blink Chromium code I've used (with a few high-profile exceptions), we avoid using non-const refs pretty much anywhere.  Because of these factors, I ask authors to avoid non-const refs, and convert code not to use them when I encounter it.

PK

[dan...@chromium.org]

Interesting, I've never thought that rules about arguments could apply to return values. Is this a practice followed in google3?

The reasoning for not passing arguments by non-const ref is that the caller can't tell that their lvalue thing is going to be modified. This isn't something relevant for returns though.

 

PK

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

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFCauk3GYLoYj1u0odfVAFg6MU00skG4auZeCHBKsJRqfA%40mail.gmail.com.

[Peter Kasting]

On Tue, Jan 31, 2017 at 1:08 PM, <dan...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 4:04 PM, 'Peter Kasting' via cxx <c...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 12:57 PM, 'Dmitry Skiba' via cxx <c...@chromium.org> wrote:

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

From http://google.github.io/styleguide/cppguide.html#Reference_Arguments :

"it is a very strong convention in Google code that input arguments are values or const references while output arguments are pointers"

The rest of that section is slightly ambiguous, because it speaks strongly of disallowing non-const refs as parameters, but it mostly sounds like it's speaking about input parameters, except for this sentence that speaks about "output arguments".

However, at the least, I think this suggests preferring return-by-pointer, even if it's not mandated.  In most non-Blink Chromium code I've used (with a few high-profile exceptions), we avoid using non-const refs pretty much anywhere.  Because of these factors, I ask authors to avoid non-const refs, and convert code not to use them when I encounter it.

Interesting, I've never thought that rules about arguments could apply to return values. Is this a practice followed in google3?

I don't work in google3, so the quote about "output arguments are pointers" being a "strong convention in Google code" is the only information I have.

The reasoning for not passing arguments by non-const ref is that the caller can't tell that their lvalue thing is going to be modified. This isn't something relevant for returns though.

Guessing at motivations, I would say that use of non-const refs in general (not just in function calls) leads to code where it's harder to see that object modifications are happening, so the idea is just to discourage all use of non-const refs.

PK 

[dan...@chromium.org]

On Tue, Jan 31, 2017 at 4:11 PM, Peter Kasting <pkas...@google.com> wrote:

On Tue, Jan 31, 2017 at 1:08 PM, <dan...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 4:04 PM, 'Peter Kasting' via cxx <c...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 12:57 PM, 'Dmitry Skiba' via cxx <c...@chromium.org> wrote:

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

From http://google.github.io/styleguide/cppguide.html#Reference_Arguments :

"it is a very strong convention in Google code that input arguments are values or const references while output arguments are pointers"

The rest of that section is slightly ambiguous, because it speaks strongly of disallowing non-const refs as parameters, but it mostly sounds like it's speaking about input parameters, except for this sentence that speaks about "output arguments".

However, at the least, I think this suggests preferring return-by-pointer, even if it's not mandated.  In most non-Blink Chromium code I've used (with a few high-profile exceptions), we avoid using non-const refs pretty much anywhere.  Because of these factors, I ask authors to avoid non-const refs, and convert code not to use them when I encounter it.

Interesting, I've never thought that rules about arguments could apply to return values. Is this a practice followed in google3?

I don't work in google3, so the quote about "output arguments are pointers" being a "strong convention in Google code" is the only information I have.

But output arguments != return values. Output arguments are arguments that the function modifies.

A query for "^\ *[a-zA-Z0-9:]+& -operator" finds almost a million instances, so I don't think this is supposed to apply to return values.

 

The reasoning for not passing arguments by non-const ref is that the caller can't tell that their lvalue thing is going to be modified. This isn't something relevant for returns though.

Guessing at motivations, I would say that use of non-const refs in general (not just in function calls) leads to code where it's harder to see that object modifications are happening, so the idea is just to discourage all use of non-const refs.

Calling methods on a non-const-object instead of a non-const-pointer does not seem more ambiguous.

[Peter Kasting]

On Tue, Jan 31, 2017 at 1:17 PM, <dan...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 4:11 PM, Peter Kasting <pkas...@google.com> wrote:

On Tue, Jan 31, 2017 at 1:08 PM, <dan...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 4:04 PM, 'Peter Kasting' via cxx <c...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 12:57 PM, 'Dmitry Skiba' via cxx <c...@chromium.org> wrote:

BTW, what does our code style say of

Foo* GetFoo() {

  static Foo* foo = new Foo();

  return foo;

}

vs

Foo& GetFoo() {

  static Foo* foo = new Foo();

  return *foo;

}

From http://google.github.io/styleguide/cppguide.html#Reference_Arguments :

"it is a very strong convention in Google code that input arguments are values or const references while output arguments are pointers"

The rest of that section is slightly ambiguous, because it speaks strongly of disallowing non-const refs as parameters, but it mostly sounds like it's speaking about input parameters, except for this sentence that speaks about "output arguments".

However, at the least, I think this suggests preferring return-by-pointer, even if it's not mandated.  In most non-Blink Chromium code I've used (with a few high-profile exceptions), we avoid using non-const refs pretty much anywhere.  Because of these factors, I ask authors to avoid non-const refs, and convert code not to use them when I encounter it.

Interesting, I've never thought that rules about arguments could apply to return values. Is this a practice followed in google3?

I don't work in google3, so the quote about "output arguments are pointers" being a "strong convention in Google code" is the only information I have.

But output arguments != return values. Output arguments are arguments that the function modifies.

Oh, is that what they meant by that phrasing?  That would indeed make sense... I feel silly for misreading it all this time.

A query for "^\ *[a-zA-Z0-9:]+& -operator" finds almost a million instances,

In google3, I assume :).  I see about 2100 instances in Chrome (not counting V8 or third_party), which ranks as "uncommon but not entirely unheard of" for our size.

 

so I don't think this is supposed to apply to return values.

Yes, I agree.

Hmm.  This suggests I should be less hostile to non-const refs in code I encounter.

PK

[Brett Wilson]

On Tue, Jan 31, 2017 at 12:15 PM, Scott Graham <sco...@chromium.org> wrote:

I believe we should:

- try to not use LazyInstance in new code;

- replace code that uses LazyInstance where it could be replaced by less code (this seems to apply especially to LazyInstance::Leaky);

Doesn't a static call a destructor on shutdown? Was this discussed and I missed it? It doesn't seem like we want to add these static destructors.

Brett

[Peter Kasting]

On Tue, Jan 31, 2017 at 1:28 PM, Brett Wilson <bre...@chromium.org> wrote:

On Tue, Jan 31, 2017 at 12:15 PM, Scott Graham <sco...@chromium.org> wrote:

I believe we should:

- try to not use LazyInstance in new code;

- replace code that uses LazyInstance where it could be replaced by less code (this seems to apply especially to LazyInstance::Leaky);

Doesn't a static call a destructor on shutdown?

Not with CR_DEFINE_STATIC_LOCAL.

PK

[Scott Graham]

On Tue, Jan 31, 2017 at 1:28 PM, Brett Wilson <bre...@chromium.org> wrote:

Yes, a regular object would by default. All LazyInstances are pointers already, so I was implicitly thinking of static raw pointers, not just any old static. AFAIK, there shouldn't be any startup or shutdown code in that case.

 

Brett

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

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABiGVV-%3DN8fjr0%3DXPXYqwXztP0GNAd1Foc%3DYC7jzKzLROWZpNQ%40mail.gmail.com.

[Mark Mentovai]

After converting and looking at a few of these, I’ve found that the pointer variant has felt like a more natural fit in the majority of situations. The reference version was handy in a few instances where the existing code needed a reference anyway, and it made more sense to convert from pointer to reference centrally in the helper function instead of at each use site. I don’t think that we should express any preference.

[Mark Mentovai]

We’re not doing

Snooze* GetSnooze() {

  static Snooze s;

  return &s;

}

We’re doing

Snooze* GetSnooze() {

  static Snooze* s = new Snooze();

  return s;

}

Nothing registers any at-exit code in this case.

[Brett Wilson]

On Tue, Jan 31, 2017 at 1:40 PM, Mark Mentovai <ma...@chromium.org> wrote:

We’re not doing

Snooze* GetSnooze() {

  static Snooze s;

  return &s;

}

We’re doing

Snooze* GetSnooze() {

  static Snooze* s = new Snooze();

  return s;

}

Nothing registers any at-exit code in this case.

Ah, sounds good to me.

Brett

[Yuzhu Shen]

Returning a reference is nice in the case whether the object is never null:

  Foo& foo();

v.s.

  // Hey users. The return pointer is never null. So you don't need to do null-check.

  Foo* foo();

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHk1GDz6ngr4oA%2BGQ%2B46URzRBP2E%3Did0S1fMmaNuMgkcLtnMQA%40mail.gmail.com.

[Yuzhu Shen]

On Tue, Jan 31, 2017 at 2:08 PM, Yuzhu Shen <yzs...@chromium.org> wrote:

Returning a reference is nice in the case whether the object is never null:

"whether" -> "where"

[Greg Thompson]

On Tue, Jan 31, 2017 at 11:09 PM Yuzhu Shen <yzs...@chromium.org> wrote:

Returning a reference is nice in the case whether the object is never null:

  Foo& foo();

v.s.

  // Hey users. The return pointer is never null. So you don't need to do null-check.

  Foo* foo();

This has been my reason for preferring return by non-const reference. Otherwise, it's understandable for someone to DCHECK the return value, which I would say is an anti-pattern as it clutters up the points of use. If these functions ever return nullptr, the house is on fire and a DCHECK isn't going to put it out.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABxY52A5MFi2R8a%3DSArWRhOZz4QN_ovFpq7uk5Pikt%3DNty-V%3Dg%40mail.gmail.com.

[Sidney San Martín]

On Tue, Jan 31, 2017 at 5:08 PM, Yuzhu Shen <yzs...@chromium.org> wrote:

Returning a reference is nice in the case where the object is never null:

  Foo& foo();

v.s.

  // Hey users. The return pointer is never null. So you don't need to do null-check.

  Foo* foo();

+1. I'm in the middle of removing the possibility of nulls from some code right now, and changing functions to return references (const, in my case) has helped me find a bunch of now-obsolete null checks.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABxY52A5MFi2R8a%3DSArWRhOZz4QN_ovFpq7uk5Pikt%3DNty-V%3Dg%40mail.gmail.com.

[Anthony Berent]

I have always worried about code that assumes that refs can't be null. The following is, in my understanding, entirely legal:

#include <stdio.h>

int &test() {

    int* j=nullptr;

    return *j;

}

int main() {

    int& i = test();

    printf("&i = %p\n", &i);

}

On compiling and running it it prints:

&i = (nil)

Yes, it is a very good convention to say that we will never return null through a reference; but please remember that it is only a convention, and is not enforced by the language.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHPGDUYG1OkvGUe%3DTWTuhuVpKtDSb7RMEWSLgJD9tQBVv-SQeg%40mail.gmail.com.

[Tomasz Śniatowski]

That'd be illegal / UB, as defined by the language (and as caught by -fsanitize=undefined).

8.3.2/5: "[...] A reference shall be initialized to refer to a valid object or function. [ Note: in particular, a null reference cannot exist in a well-defined program, because the only way to create such a reference would be to bind it to the “object” obtained by indirection through a null pointer, which causes undefined behavior [...]"

clang: "runtime error: reference binding to null pointer of type 'int'"

--
Tomasz Śniatowski

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABdZ6yApuN5CvKNnEe4t268LpiZAA5kYFo-joWhxNUo-SMCo%3DQ%40mail.gmail.com.