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 <tarmstr...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to