Implemented aligning and poisoning global redzones

Implemented aligning and poisoning global redzones

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
R=kschimpf@google.com, stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=aedc5e49ded4595d723b09ee31a16884ced06680

[tlively, 2016-06-29 05:34:32]

tlively@google.com changed reviewers:
+ kschimpf@google.com, mcgrathr@chromium.org, stichnot@chromium.org

[tlively, 2016-06-29 05:34:33]

A simple hello world program currently crashes due to an out of bounds access of
a global in the runtime, so I will have to track it down and implement an
instrumentation blacklist. The builds and tests all pass, though, so that will
be in the next CL.

[Jim Stichnoth, 2016-06-29 14:08:06]

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:34: const std::string RzPrefix = "__$rz";
If possible, change these "const std::string" to "constexpr char *".

This should reduce the number of static initializations in Subzero.

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:49: for (unsigned i = 0; i < sizeof(Size); i++) {
Use pre-increment instead of post-increment when possible.

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.h (right):

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.h:45: /*
Why is this declaration (and its corresponding implementation) commented out?

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.h:62: SizeT RzGlobalsNum;
SizeT RzGlobalsNum = 0;

then you can remove it from the initializer list in the ctor.

[tlively, 2016-06-29 16:38:31]

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:34: const std::string RzPrefix = "__$rz";
On 2016/06/29 14:08:06, stichnot wrote:
> If possible, change these "const std::string" to "constexpr char *".
> 
> This should reduce the number of static initializations in Subzero.

Done.

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:49: for (unsigned i = 0; i < sizeof(Size); i++) {
On 2016/06/29 14:08:06, stichnot wrote:
> Use pre-increment instead of post-increment when possible.

Done.

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.h (right):

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.h:45: /*
On 2016/06/29 14:08:06, stichnot wrote:
> Why is this declaration (and its corresponding implementation) commented out?

Oops. Done.

https://codereview.chromium.org/2108083002/diff/20001/src/IceASanInstrumentat...
src/IceASanInstrumentation.h:62: SizeT RzGlobalsNum;
On 2016/06/29 14:08:06, stichnot wrote:
> SizeT RzGlobalsNum = 0;
> 
> then you can remove it from the initializer list in the ctor.

Done.

[Karl, 2016-06-30 14:23:31]

lgtm

[Jim Stichnoth, 2016-06-30 14:40:31]

lgtm

https://codereview.chromium.org/2108083002/diff/40001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2108083002/diff/40001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:34: constexpr char *RzPrefix = (char*) "__$rz";
Is the char* cast really needed?

If so, can you at least make it "const char *" ?

[tlively, 2016-06-30 18:06:15]

Description was changed from

==========
Implemented aligning and poisoning global redzones

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
==========

to

==========
Implemented aligning and poisoning global redzones

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
R=kschimpf@google.com, stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[tlively, 2016-06-30 18:06:17]

Committed patchset #5 (id:80001) manually as
aedc5e49ded4595d723b09ee31a16884ced06680 (presubmit successful).

[tlively, 2016-06-30 18:07:17]

https://codereview.chromium.org/2108083002/diff/40001/src/IceASanInstrumentat...
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2108083002/diff/40001/src/IceASanInstrumentat...
src/IceASanInstrumentation.cpp:34: constexpr char *RzPrefix = (char*) "__$rz";
On 2016/06/30 14:40:30, stichnot wrote:
> Is the char* cast really needed?
> 
> If so, can you at least make it "const char *" ?

Done.