You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2017/08/04 21:42:48 UTC
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Henry Robinson has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7591
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
IMPALA-5666: ASAN poisoning for MemPool and BufferPool
* Use ASAN_[UN]POISON_MEMORY_REGION to indicate to ASAN runtime that
memory is not 'valid' from Impala's perspective (but still valid from
kernel's perspective).
* Add this to MemPool and BufferPoolAllocator. For both, memory goes
through the following lifecycle: when it is allocated from the OS and
returned to the user, it must be unpoisoned. When the user returns it
to a cache, it becomes poisoned. When the cache is freed to the OS, it
must be unpoisoned again (otherwise future allocations elsewhere in
the system might return poisoned memory).
* Also add checks to FreePool which uses a MemPool underneath, but has
its own freelist cache. Fix a couple of bugs in free-pool-test that
these checks uncovered.
Testing:
* Tests that only run if ASAN is enabled to confirm that poisoning
catches simple use-after-return errors. These are 'death' tests which
catch expected process exits.
Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
---
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/free-list.h
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
10 files changed, 152 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/7591/1
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1046/
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#2).
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
IMPALA-5666: ASAN poisoning for MemPool and BufferPool
"If you poison us, do we not die?"
* Use ASAN_[UN]POISON_MEMORY_REGION to indicate to ASAN runtime that
memory is not 'valid' from Impala's perspective (but still valid from
kernel's perspective).
* Add this to MemPool and BufferPoolAllocator. For both, memory goes
through the following lifecycle: when it is allocated from the OS and
returned to the user, it must be unpoisoned. When the user returns it
to a cache, it becomes poisoned. When the cache is freed to the OS, it
must be unpoisoned again (otherwise future allocations elsewhere in
the system might return poisoned memory).
* Also add checks to FreePool which uses a MemPool underneath, but has
its own freelist cache. Fix a couple of bugs in free-pool-test that
these checks uncovered.
Testing:
* Tests that only run if ASAN is enabled to confirm that poisoning
catches simple use-after-return errors. These are 'death' tests which
catch expected process exits.
* Fix one bug found in StringFunctions::FindInSet which would read one
byte past the end of a stringvalue.
Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
---
M be/src/exprs/string-functions-ir.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/free-list.h
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
10 files changed, 146 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/7591/2
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7591/1//COMMIT_MSG
Commit Message:
Line 24: these checks uncovered.
> Did you run the full test suite with ASAN?
Not yet, will do (and update) before I commit. Have run a good subset of the be-tests and some full queries.
http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-allocator.cc
File be/src/runtime/bufferpool/buffer-allocator.cc:
Line 400: // Ensure that the memory is unpoisoned when it's next allocated by the system.
> Is this one needed? Won't the system allocator call free(), which will pois
You would have thought so, but I thought I saw one case where memory returned to the system was re-allocated as poisoned. Better safe than sorry.
http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:
Line 414: void Poison() { ASAN_POISON_MEMORY_REGION(data(), len()); }
> I doubt it makes much of a difference but it would be nice if we could make
Done
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1046/
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 2:
Thanks! Unfortunately there's an issue with the ASAN build - the huge group_concat query in our test suite takes over 12 hours to run because of all the poisoning...
I'm going to look briefly into whether the query has a pessimal allocation pattern that we can fix.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 3:
BE and EE tests pass with ASAN enabled with this patch.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7591
to look at the new patch set (#3).
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
IMPALA-5666: ASAN poisoning for MemPool and BufferPool
"If you poison us, do we not die?"
* Use ASAN_[UN]POISON_MEMORY_REGION to indicate to ASAN runtime that
memory is not 'valid' from Impala's perspective (but still valid from
kernel's perspective).
* Add this to MemPool and BufferPoolAllocator. For both, memory goes
through the following lifecycle: when it is allocated from the OS and
returned to the user, it must be unpoisoned. When the user returns it
to a cache, it becomes poisoned. When the cache is freed to the OS, it
must be unpoisoned again (otherwise future allocations elsewhere in
the system might return poisoned memory).
* Also add checks to FreePool which uses a MemPool underneath, but has
its own freelist cache. Fix a couple of bugs in free-pool-test that
these checks uncovered.
Testing:
* Tests that only run if ASAN is enabled to confirm that poisoning
catches simple use-after-return errors. These are 'death' tests which
catch expected process exits.
Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
---
M be/CMakeLists.txt
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/free-list.h
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
10 files changed, 189 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/7591/3
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1047/
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 2: Code-Review+2
I got my wires crossed and thought I was waiting for a new patchset here. I'm clearly not. This looks great. Seems fine to merge once we get a clean ASAN build.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 1:
(4 comments)
Had a few very minor comments. This should be very helpful in catching any use-after-free bugs.
http://gerrit.cloudera.org:8080/#/c/7591/1//COMMIT_MSG
Commit Message:
Line 7: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
The JIRA number and name are a bit ominous
Line 24: Testing:
Did you run the full test suite with ASAN?
http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-allocator.cc
File be/src/runtime/bufferpool/buffer-allocator.cc:
Line 400: buffer.Unpoison();
Is this one needed? Won't the system allocator call free(), which will poison it anyway? Or is my understanding faulty?
http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:
Line 414: void Poison();
I doubt it makes much of a difference but it would be nice if we could make this an inline function and optimise it out for non-ASAN builds.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 3:
This patch fixes two issues over the previous patch:
1. Fix quadratic behaviour when reallocating from a free pool - previously the patch would poison the entire chunk, and then unpoison the allocated bytes. We can do better by tracking the size of the allocation, and then only unpoisoning the bytes that are newly allocated after the reallocate call (or poisoning them if the allocation shrinks). This fixes an issue where test_very_large_strings would take hours and hours to execute: it now takes about 6 minutes on my machine.
2. ADDRESS_SANITIZER was not defined in codegen-compiled code. Added -DADDRESS_SANITIZER to CMAKE_CXX_IR_FLAGS.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
Patch Set 2:
Just an update - now that I run this on the full set of tests, it finds some quite interesting, and real-but-dormant, bugs. I'll fix the ones that are straightforward (found two so far), but if it gets out of hand I'll file JIRAs and share the work around.
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has submitted this change and it was merged.
Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
......................................................................
IMPALA-5666: ASAN poisoning for MemPool and BufferPool
"If you poison us, do we not die?"
* Use ASAN_[UN]POISON_MEMORY_REGION to indicate to ASAN runtime that
memory is not 'valid' from Impala's perspective (but still valid from
kernel's perspective).
* Add this to MemPool and BufferPoolAllocator. For both, memory goes
through the following lifecycle: when it is allocated from the OS and
returned to the user, it must be unpoisoned. When the user returns it
to a cache, it becomes poisoned. When the cache is freed to the OS, it
must be unpoisoned again (otherwise future allocations elsewhere in
the system might return poisoned memory).
* Also add checks to FreePool which uses a MemPool underneath, but has
its own freelist cache. Fix a couple of bugs in free-pool-test that
these checks uncovered.
Testing:
* Tests that only run if ASAN is enabled to confirm that poisoning
catches simple use-after-return errors. These are 'death' tests which
catch expected process exits.
Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Reviewed-on: http://gerrit.cloudera.org:8080/7591
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/free-list.h
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
10 files changed, 189 insertions(+), 31 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Henry Robinson: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>