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