MSan: mark any memory allocated from the JS heap as uninitialized.

MSan: mark any memory allocated from the JS heap as uninitialized.

BUG=chromium:403409, chromium:178409
R=jkummerow@chromium.org
LOG=N

Committed: https://code.google.com/p/v8/source/detail?r=23268

[earthdok, 2014-08-20 15:23:40]

Please take a look (assuming the general approach makes sense, as discussed on
crbug).

[Jakob Kummerow, 2014-08-21 08:13:46]

LGTM, we can give this a shot.

Considering that MSan is only run with simulators, this approach shouldn't
introduce false positives due to missing unpoisoning. Have you run it locally to
see if it reports anything?

The other caveat I mentioned on the bug is still valid, though: fast inline
allocation from generated code isn't covered by this. But some coverage is
better than no coverage, so this limitation is not a blocker.

https://codereview.chromium.org/480763003/diff/1/src/heap/heap-inl.h
File src/heap/heap-inl.h (left):

https://codereview.chromium.org/480763003/diff/1/src/heap/heap-inl.h#oldcode19
src/heap/heap-inl.h:19: 
accidental edit?

[earthdok, 2014-08-21 09:15:13]

On 2014/08/21 08:13:46, Jakob wrote:
> LGTM, we can give this a shot.
> 
> Considering that MSan is only run with simulators, this approach shouldn't
> introduce false positives due to missing unpoisoning. Have you run it locally
to
> see if it reports anything?
I've only verified that the reproducer in issue 403409 causes an MSan report
after this change. I've not done much to check that no false positives have been
introduced.

By the way, I think the annotations in msan.h under !defined(USE_SIMULATOR) can
be removed, as we don't have any plans to support that mode of usage. I'll wait
for an OK from eugenis@ and then nuke them.

> The other caveat I mentioned on the bug is still valid, though: fast inline
> allocation from generated code isn't covered by this. But some coverage is
> better than no coverage, so this limitation is not a blocker.
> 
> https://codereview.chromium.org/480763003/diff/1/src/heap/heap-inl.h
> File src/heap/heap-inl.h (left):
> 
> https://codereview.chromium.org/480763003/diff/1/src/heap/heap-inl.h#oldcode19
> src/heap/heap-inl.h:19: 
> accidental edit?
Fixed, thanks.

[earthdok, 2014-08-21 09:35:33]

The CQ bit was checked by earthdok@chromium.org

[commit-bot: I haz the power, 2014-08-21 09:35:54]

CQ is trying da patch. Follow status at
 https://v8-status.appspot.com/cq/earthdok@chromium.org/480763003/20001

[commit-bot: I haz the power, 2014-08-21 09:36:02]

Committed patchset #2 (20001) as 23268

[Evgeniy Stepanov, 2014-08-26 08:57:27]

On 2014/08/21 09:15:13, earthdok wrote:
> On 2014/08/21 08:13:46, Jakob wrote:
> > LGTM, we can give this a shot.
> > 
> > Considering that MSan is only run with simulators, this approach shouldn't
> > introduce false positives due to missing unpoisoning. Have you run it
locally
> to
> > see if it reports anything?
> I've only verified that the reproducer in issue 403409 causes an MSan report
> after this change. I've not done much to check that no false positives have
been
> introduced.
> 
> By the way, I think the annotations in msan.h under !defined(USE_SIMULATOR)
can
> be removed, as we don't have any plans to support that mode of usage. I'll
wait
> for an OK from eugenis@ and then nuke them.

This sounds like a good idea.