Add a hard limit for Flash JIT pages (issue 9442002)

[jsc...@chromium.org]

Reviewers: cpu, ananta,

Message:
Adding a hard limit. I'll monitor canary and if crashes are high I'll scale
it
back.

Description:
Add a hard limit for Flash JIT pages

This includes a unique exception signature that I will monitor for crashes:
http://crash/search?query=crash_reason%3A%22EXCEPTION_ACCESS_VIOLATION_WRITE%22+crash_address%3A%222880249322%22+product%3A%22Chrome%22

BUG=113891

Please review this at http://codereview.chromium.org/9442002/

SVN Base: svn://chrome-svn/chrome/trunk/src/

Affected files:
M webkit/plugins/npapi/webplugin_delegate_impl_win.cc

Index: webkit/plugins/npapi/webplugin_delegate_impl_win.cc
===================================================================
--- webkit/plugins/npapi/webplugin_delegate_impl_win.cc (revision 122838)
+++ webkit/plugins/npapi/webplugin_delegate_impl_win.cc (working copy)
@@ -148,7 +148,7 @@
SIZE_T size,
DWORD free_type);

-const size_t kMaxPluginExecMemSize = 64 * 1024 * 1024; // 64mb.
+const size_t kMaxPluginExecMemSize = 16 * 1024 * 1024; // 16mb.
const DWORD kExecPageMask = PAGE_EXECUTE | PAGE_EXECUTE_READ |
PAGE_EXECUTE_READWRITE;
static volatile intptr_t g_max_exec_mem_size;
@@ -167,6 +167,13 @@
return g_exec_mem_size;
}

+// Throw a unique exception when the JIT limit is hit.
+inline void RaiseJITException() {
+ static const ULONG parameters[] = {1, 0xabad1dea /* 2880249322 */ };
+ ::RaiseException(EXCEPTION_ACCESS_VIOLATION, EXCEPTION_NONCONTINUABLE,
+ 2, parameters);
+}
+
// http://crbug.com/16114
// Enforces providing a valid device context in NPWindow, so that
NPP_SetWindow
// is never called with NPNWindoTypeDrawable and NPWindow set to NULL.
@@ -357,10 +364,8 @@
if (size && p && (protect & kExecPageMask)) {
bool limit_exceeded = UpdateExecMemSize(static_cast<intptr_t>(size)) >
kMaxPluginExecMemSize;
-#ifndef NDEBUG // TODO(jschuh): Do this in release after we get numbers.
if (limit_exceeded)
- ::DebugBreak();
-#endif
+ RaiseJITException();
}
return p;
}
@@ -375,10 +380,8 @@
if (is_exec && !was_exec) {
bool limit_exceeded = UpdateExecMemSize(static_cast<intptr_t>(size))
>
kMaxPluginExecMemSize;
-#ifndef NDEBUG // TODO(jschuh): Do this in release after we get numbers.
if (limit_exceeded)
- ::DebugBreak();
-#endif
+ RaiseJITException();
} else if (!is_exec && was_exec) {
UpdateExecMemSize(-(static_cast<intptr_t>(size)));
}

[c...@chromium.org]

lgtm

http://codereview.chromium.org/9442002/diff/1/webkit/plugins/npapi/webplugin_delegate_impl_win.cc
File webkit/plugins/npapi/webplugin_delegate_impl_win.cc (right):

http://codereview.chromium.org/9442002/diff/1/webkit/plugins/npapi/webplugin_delegate_impl_win.cc#newcode174
webkit/plugins/npapi/webplugin_delegate_impl_win.cc:174: 2, parameters);
instead of 2 use arraysize of whatever it is called

http://codereview.chromium.org/9442002/

[c...@chromium.org]

reminder to remove the VirtualAlloc patch

http://codereview.chromium.org/9442002/

[ana...@chromium.org]

LGTM

http://codereview.chromium.org/9442002/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jsc...@chromium.org/9442002/1

http://codereview.chromium.org/9442002/

[commi...@chromium.org]

Try job failure for 9442002-1 (retry) on win_rel for steps "browser_tests,
ui_tests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=3512

http://codereview.chromium.org/9442002/

[commi...@chromium.org]

[commi...@chromium.org]

Change committed as 123205

http://codereview.chromium.org/9442002/