Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(284)

Issue 2744002: Annotate PageAllocator for MSan.

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 4 months ago by earthdok
Modified:
3 years, 3 months ago
Reviewers:
Mark Mentovai
Base URL:
https://chromium.googlesource.com/external/google-breakpad/src.git@master
Visibility:
Public.

Description

Annotate PageAllocator for MSan.

PageAllocator maps memory via sys_mmap(), implemented in
linux_syscall_support.h. We need to explicitly inform MSan that this memory is
initialized.

BUG=chromium:394028
R=mark@chromium.org
TEST=compile

Patch Set 1 #

Patch Set 2 : typo #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M common/memory.h View 1 2 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 6
earthdok
please take a look
3 years, 4 months ago #1
Mark Mentovai
LGTM
3 years, 4 months ago #2
earthdok
On 2014/07/22 12:32:27, Mark Mentovai wrote: > LGTM Could you please land this?
3 years, 4 months ago #3
Mark Mentovai
Committed Breakpad r1356.
3 years, 4 months ago #4
earthdok
On 2014/07/22 14:24:38, Mark Mentovai wrote: > Committed Breakpad r1356. Thanks!
3 years, 4 months ago #5
earthdok
3 years, 3 months ago #6
For the record:

this did not fix the spurious MSan report. It was actually caused by the fact
that components/breakpad/app/breakpad_linux.cc passes to PageAllocator::Alloc()
a |bytes| argument which is obtained from sys_fstat(). (And thus |bytes| is
poisoned when we check it in Alloc()).  Alas, I couldn't reproduce the issue on
my machine so there was no way to verify the fix.

As for whether this CL is actually necessary, it depends on whether there is any
code at all that makes the assumption that those particular mmap calls zero-fill
the memory. Users of this allocator probably shouldn't make that assumption (at
least it's not documented in the interface), but the allocator's implementation
could conceivably do that. I'm going to err on the side of caution and keep
those annotations.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted