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

48 views
Skip to first unread message

jsc...@chromium.org

unread,
Feb 22, 2012, 6:42:14 PM2/22/12
to c...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@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

unread,
Feb 22, 2012, 8:06:10 PM2/22/12
to jsc...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@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

unread,
Feb 22, 2012, 8:18:01 PM2/22/12
to jsc...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@chromium.org
reminder to remove the VirtualAlloc patch

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

ana...@chromium.org

unread,
Feb 22, 2012, 9:18:08 PM2/22/12
to jsc...@chromium.org, c...@chromium.org, chromium...@chromium.org, dari...@chromium.org

commi...@chromium.org

unread,
Feb 22, 2012, 10:13:11 PM2/22/12
to jsc...@chromium.org, c...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@chromium.org

commi...@chromium.org

unread,
Feb 23, 2012, 1:33:06 AM2/23/12
to jsc...@chromium.org, c...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@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

unread,
Feb 23, 2012, 1:39:51 AM2/23/12
to jsc...@chromium.org, c...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@chromium.org

commi...@chromium.org

unread,
Feb 23, 2012, 4:42:34 AM2/23/12
to jsc...@chromium.org, c...@chromium.org, ana...@chromium.org, chromium...@chromium.org, dari...@chromium.org
Change committed as 123205

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

Reply all
Reply to author
Forward
0 new messages