Ensure Freed TLS Slots Contain nullptr on Reallocation

Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907
Committed: https://crrev.com/c90ffe2a6c10bb34c8eed5225f6d3e4d3be23acb
Cr-Commit-Position: refs/heads/master@{#423952}

[robliao, 2016-09-30 23:01:08]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 23:02:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-01 00:13:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-01 00:13:03]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[robliao, 2016-10-03 17:19:57]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-03 17:20:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2016-10-03 17:21:26]

robliao@chromium.org changed reviewers:
+ brettw@chromium.org

[robliao, 2016-10-03 17:21:26]

Please review this CL. Thanks!

[commit-bot: I haz the power, 2016-10-03 18:49:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-03 18:49:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[brettw, 2016-10-03 20:05:16]

There's something about this system I'm not following that's making this not
make sense to me. Since this is in your head, I wonder if you can put a
conceptual overview of how our TLS system works at the top of the .cc file: how
our system interacts with the OS one, which structure are per-thread and which
structures are global. Then I can read that and hopefully be able to tell that
this is correct :)

I think my confusion is around how the version interacts with the global and
per-thread data.

[robliao, 2016-10-04 00:06:04]

On 2016/10/03 20:05:16, brettw (ping on IM after 24h) wrote:
> There's something about this system I'm not following that's making this not
> make sense to me. Since this is in your head, I wonder if you can put a
> conceptual overview of how our TLS system works at the top of the .cc file:
how
> our system interacts with the OS one, which structure are per-thread and which
> structures are global. Then I can read that and hopefully be able to tell that
> this is correct :)
> 
> I think my confusion is around how the version interacts with the global and
> per-thread data.

Done!

[robliao, 2016-10-04 00:06:20]

Description was changed from

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

BUG=590907
==========

to

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907
==========

[robliao, 2016-10-04 00:07:41]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 00:08:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 01:46:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 01:46:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[brettw, 2016-10-04 03:33:57]

lgtm

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
File base/threading/thread_local_storage.cc (right):

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
base/threading/thread_local_storage.cc:15: // Chrome Thread Local Storage (TLS)
This comment is awesome! Thanks.

[robliao, 2016-10-04 19:19:53]

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
File base/threading/thread_local_storage.cc (right):

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
base/threading/thread_local_storage.cc:15: // Chrome Thread Local Storage (TLS)
On 2016/10/04 03:33:57, brettw (ping on IM after 24h) wrote:
> This comment is awesome! Thanks.

Thanks. Now I just need to figure out why this fails on MacOS 10.9 and not MacOS
10.11

[robliao, 2016-10-06 00:54:31]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 00:55:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 02:39:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 02:39:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[robliao, 2016-10-06 22:14:42]

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
File base/threading/thread_local_storage.cc (right):

https://codereview.chromium.org/2383833004/diff/20001/base/threading/thread_l...
base/threading/thread_local_storage.cc:15: // Chrome Thread Local Storage (TLS)
On 2016/10/04 19:19:52, robliao wrote:
> On 2016/10/04 03:33:57, brettw (ping on IM after 24h) wrote:
> > This comment is awesome! Thanks.
> 
> Thanks. Now I just need to figure out why this fails on MacOS 10.9 and not
MacOS
> 10.11

With the help of https://codereview.chromium.org/2395303002/ this should be able
to land.

[robliao, 2016-10-07 18:13:09]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 18:13:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 20:13:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 20:13:07]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-10-07 20:16:25]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2016-10-07 20:16:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 20:22:42]

Description was changed from

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907
==========

to

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907
==========

[commit-bot: I haz the power, 2016-10-07 20:22:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-07 20:24:37]

Description was changed from

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907
==========

to

==========
Ensure Freed TLS Slots Contain nullptr on Reallocation

Code generally depends on TLS slots initialized to zero. This means
that code that gets a reused slot also depends on the reused slot
containing zero.

This CL introduces a versioning system to allow us to quickly determine
if a slot was previously freed.

Also added overview comments as a bonus.

BUG=590907

Committed: https://crrev.com/c90ffe2a6c10bb34c8eed5225f6d3e4d3be23acb
Cr-Commit-Position: refs/heads/master@{#423952}
==========

[commit-bot: I haz the power, 2016-10-07 20:24:38]

Patchset 2 (id:??) landed as
https://crrev.com/c90ffe2a6c10bb34c8eed5225f6d3e4d3be23acb
Cr-Commit-Position: refs/heads/master@{#423952}