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