Remove the is_nested bit from the BlockInfo structure.

Remove the is_nested bit from the BlockInfo structure.

This will allow us to increase the size of the |body_size| field of the
BlockHeader structure (to handle the allocations > 1GB).

We're not using this feature anyway so it's adding some unneeded complexity to our codebase.

R=chrisha@chromium.org

Committed: https://github.com/google/syzygy/commit/a1f6aa62493450075ae4a6e5a2a4137d26174b40

[Sébastien Marchand, 2016-11-17 21:52:51]

Description was changed from

==========
Remove the is_nested bit from the BlockInfo structure.

This will allow us to increase the size of the |body_size| field of the
BlockHeader structure (to handle the allocations > 1GB).
==========

to

==========
Remove the is_nested bit from the BlockInfo structure.

This will allow us to increase the size of the |body_size| field of the
BlockHeader structure (to handle the allocations > 1GB).

We're not using this feature anyway so it's adding some unneeded complexity to
our codebase.
==========

[Sébastien Marchand, 2016-11-17 21:52:51]

sebmarchand@chromium.org changed reviewers:
+ chrisha@chromium.org, siggi@chromium.org

[Sébastien Marchand, 2016-11-22 17:52:55]

PTAL.

[Sigurður Ásgeirsson, 2016-11-22 19:15:37]

got some nits, deferring to Chris on reviewing actual functionality

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/block.cc
File syzygy/agent/asan/block.cc (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/block.cc#...
syzygy/agent/asan/block.cc:467: sizeof(BlockInfo) - FIELD_OFFSET(BlockInfo,
block_pages);
I'm not sure this is right - how about field_offset(last member) +
sizeof(last_member)?

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow.h
File syzygy/agent/asan/shadow.h (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow.h#...
syzygy/agent/asan/shadow.h:17: // The layout of a block is fully encoded in
shadow memory, allowing for
maybe this comment can be relocated to a .md file (in a separate CL)?

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
File syzygy/agent/asan/shadow_marker.cc (left):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
syzygy/agent/asan/shadow_marker.cc:26: static const uint8_t kBlockEndNestedBit =
0x01;
I don't feel qualified to review this file.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
File syzygy/agent/asan/shadow_marker.h (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
syzygy/agent/asan/shadow_marker.h:266: static ShadowMarker BuildBlockEnd(bool
active);
nit: indent

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_un...
File syzygy/agent/asan/shadow_unittest.cc (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_un...
syzygy/agent/asan/shadow_unittest.cc:511: void TestBlockInfoFromShadow(Shadow*
shadow, const BlockLayout& outer) {
nit: rename outer to something more descriptive?

[Sébastien Marchand, 2016-11-22 22:12:23]

Thanks. Chris: PTAL.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/block.cc
File syzygy/agent/asan/block.cc (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/block.cc#...
syzygy/agent/asan/block.cc:467: sizeof(BlockInfo) - FIELD_OFFSET(BlockInfo,
block_pages);
On 2016/11/22 19:15:37, Sigurður Ásgeirsson wrote:
> I'm not sure this is right - how about field_offset(last member) +
> sizeof(last_member)?

"sizeof(BlockInfo) - FIELD_OFFSET(BlockInfo, block_pages)" was working fine
because |is_nested| used to be the last field of the struct (and so
sizeof(BlockInfo) is now equal to what ELD_OFFSET(BlockInfo, is_nested) was
evaluating to), but your suggestion is probably safer.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow.h
File syzygy/agent/asan/shadow.h (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow.h#...
syzygy/agent/asan/shadow.h:17: // The layout of a block is fully encoded in
shadow memory, allowing for
On 2016/11/22 19:15:37, Sigurður Ásgeirsson wrote:
> maybe this comment can be relocated to a .md file (in a separate CL)?

Maybe, we don't really use .md files (yet) so if we move this one then we should
probably move more documentation at the same time, not sure if we'll have the
bandwidth for this.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
File syzygy/agent/asan/shadow_marker.cc (left):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
syzygy/agent/asan/shadow_marker.cc:26: static const uint8_t kBlockEndNestedBit =
0x01;
On 2016/11/22 19:15:37, Sigurður Ásgeirsson wrote:
> I don't feel qualified to review this file.

IIRC Chris wrote most of it.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
File syzygy/agent/asan/shadow_marker.h (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_ma...
syzygy/agent/asan/shadow_marker.h:266: static ShadowMarker BuildBlockEnd(bool
active);
On 2016/11/22 19:15:37, Sigurður Ásgeirsson wrote:
> nit: indent

Done. Not sure why 'git cl format' doesn't like this code.

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_un...
File syzygy/agent/asan/shadow_unittest.cc (right):

https://codereview.chromium.org/2508333002/diff/1/syzygy/agent/asan/shadow_un...
syzygy/agent/asan/shadow_unittest.cc:511: void TestBlockInfoFromShadow(Shadow*
shadow, const BlockLayout& outer) {
On 2016/11/22 19:15:37, Sigurður Ásgeirsson wrote:
> nit: rename outer to something more descriptive?

Done.

[chrisha, 2016-11-23 15:15:30]

lgtm

[Sébastien Marchand, 2016-11-23 15:16:28]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2016-11-23 15:16:30]

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-11-23 15:16:40]

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

[commit-bot: I haz the power, 2016-11-23 15:16:40]

CQ has no permission to schedule in bucket tryserver.client.syzygy

[Sébastien Marchand, 2016-11-23 15:20:31]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2016-11-23 15:20: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-11-23 15:20:51]

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

[commit-bot: I haz the power, 2016-11-23 15:20:53]

CQ has no permission to schedule in bucket tryserver.client.syzygy

[Sébastien Marchand, 2016-11-23 15:21:38]

Description was changed from

==========
Remove the is_nested bit from the BlockInfo structure.

This will allow us to increase the size of the |body_size| field of the
BlockHeader structure (to handle the allocations > 1GB).

We're not using this feature anyway so it's adding some unneeded complexity to
our codebase.
==========

to

==========
Remove the is_nested bit from the BlockInfo structure.

This will allow us to increase the size of the |body_size| field of the
BlockHeader structure (to handle the allocations > 1GB).

We're not using this feature anyway so it's adding some unneeded complexity to
our codebase.

R=chrisha@chromium.org

Committed:
https://github.com/google/syzygy/commit/a1f6aa62493450075ae4a6e5a2a4137d26174b40
==========

[Sébastien Marchand, 2016-11-23 15:21:39]

Committed patchset #2 (id:20001) manually as
a1f6aa62493450075ae4a6e5a2a4137d26174b40.