Subzero: Replace pointers to allocation functions in loads

Subzero: Replace pointers to allocation functions in loads

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=48a568dbe65cd74ba688e7398f6f1f50450abfbe

[tlively, 2016-08-16 21:30:01]

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

[Jim Stichnoth, 2016-08-16 21:36:46]

lgtm

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation.cpp
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:368: if (Reloc != nullptr) {
if (auto *Reloc = llvm::dyn_cast<ConstantRelocatable>(Src)) {

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:440: // TODO(tlively): Trace back load and store
addresses to find their real
Why did this line change?

https://codereview.chromium.org/2241383006/diff/1/tests_lit/asan_tests/localr...
File tests_lit/asan_tests/localreplacement.ll (right):

https://codereview.chromium.org/2241383006/diff/1/tests_lit/asan_tests/localr...
tests_lit/asan_tests/localreplacement.ll:1: ; Test that loads local pointers to
allocation functions are instrumented
"loads of" ?

[Karl, 2016-08-16 21:38:43]

lgtm.

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation.cpp
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:382: instrumentLoad(Context, NewLoad);
Do you need to recurse here? It looks like you just need to set Inst to NewLoad
and continue.

[tlively, 2016-08-16 21:55:38]

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation.cpp
File src/IceASanInstrumentation.cpp (right):

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:368: if (Reloc != nullptr) {
On 2016/08/16 21:36:46, stichnot wrote:
> if (auto *Reloc = llvm::dyn_cast<ConstantRelocatable>(Src)) {

Done.

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:382: instrumentLoad(Context, NewLoad);
On 2016/08/16 21:38:43, Karl wrote:
> Do you need to recurse here? It looks like you just need to set Inst to
NewLoad
> and continue.

Done.

https://codereview.chromium.org/2241383006/diff/1/src/IceASanInstrumentation....
src/IceASanInstrumentation.cpp:440: // TODO(tlively): Trace back load and store
addresses to find their real
On 2016/08/16 21:36:46, stichnot wrote:
> Why did this line change?

Done.

https://codereview.chromium.org/2241383006/diff/1/tests_lit/asan_tests/localr...
File tests_lit/asan_tests/localreplacement.ll (right):

https://codereview.chromium.org/2241383006/diff/1/tests_lit/asan_tests/localr...
tests_lit/asan_tests/localreplacement.ll:1: ; Test that loads local pointers to
allocation functions are instrumented
On 2016/08/16 21:36:46, stichnot wrote:
> "loads of" ?

Done.

[tlively, 2016-08-16 21:55:53]

Description was changed from

==========
Subzero: Replace pointers to allocation functions in loads

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

to

==========
Subzero: Replace pointers to allocation functions in loads

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-08-16 21:55:54]

Committed patchset #3 (id:40001) manually as
48a568dbe65cd74ba688e7398f6f1f50450abfbe (presubmit successful).