Bug 143859

Summary: A odd crash
Product: WebKit Reporter: Peng Xinchao <xinchao.peng>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: bshafiei, fpizlo, ggaren, kling, mark.lam, msaboff, xinchao.peng, ysuzuki
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch fpizlo: review-

Description Peng Xinchao 2015-04-16 19:32:08 PDT
I happend a odd crash  : the Backtrace is below :
WebKitWebProces: unhandled page fault (11) at 0x00000005, code 0x017

gdb) bt
#0 0x9c579d5c in ?? ()
#1 0x4701ea60 in JSC::Watchdog::Scope::~Scope() () from SymbolFiles/mtd_exe/Webkit/libjavascriptcoregtk-3.0.so.0
#2 0x46e7bd00 in JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) ()
from SymbolFiles/mtd_exe/Webkit/libjavascriptcoregtk-3.0.so.0
#3 0x46f6fbb4 in JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) () from SymbolFiles/mtd_exe/Webkit/libjavascriptcoregtk-3.0.so.0
#4 0x44a6cbc0 in WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext*) () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#5 0x44a6d320 in WebCore::ScheduledAction::execute(WebCore::Document*) () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#6 0x44a6d430 in WebCore::ScheduledAction::execute(WebCore::ScriptExecutionContext*) () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#7 0x44fbc534 in WebCore::DOMTimer::fired() () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#8 0x459e1c5c in WebCore::ThreadTimers::sharedTimerFiredInternal() () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#9 0x459e1cd8 in WebCore::ThreadTimers::sharedTimerFired() () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#10 0x45fe0cf0 in WebCore::timeout_cb(void*) () from SymbolFiles/mtd_exe/Webkit/libwebkit2gtk-3.0.so.0
#11 0x43f6e8d4 in ?? ()
#12 0x43f6e8d4 in ?? ()

Welcome to guess and analysis  the reason ! Thank you
Comment 1 Mark Lam 2015-04-16 22:55:27 PDT
1. Do you have steps to reproduce this?
2. If so, what revision # are you building?
Comment 2 Peng Xinchao 2015-04-16 23:31:29 PDT
JS source code is that :

setInterval(function() {
     .......
	var $osd = $('#osd');
	$osd.show();
     setTimeout(function() {
	 $osd.hide();
       }, 5000);
    ...
     },1000);
my webkit version is webkit-r150045. platform is arm-gtk .
Comment 3 Mark Lam 2015-04-16 23:55:37 PDT
(In reply to comment #2)
> JS source code is that :
> 
> setInterval(function() {
>      .......
> 	var $osd = $('#osd');
> 	$osd.show();
>      setTimeout(function() {
> 	 $osd.hide();
>        }, 5000);
>     ...
>      },1000);
> my webkit version is webkit-r150045. platform is arm-gtk .

That is a very old version of the code.  Try updating to the tip of tree and see if the issue still manifests.
Comment 4 Peng Xinchao 2015-04-17 22:30:17 PDT
After running these  JS Source code for  20 minute , Webprocess will coredump :
<script>   
 setInterval(function() {
			
	var d = new Date();
	var time = d.getHours().addNulls() + ":" + d.getMinutes().addNulls();			
	
	var $osd = $('#osd');
	$osd.show();
	$osd.find(".channel_time").text(time);
	setTimeout(function() {
		$osd.hide();
	}, 5000);
				
    }, 10000);
Number.prototype.addNulls = function () {
    if (this < 10) {
	return "0" + this;
	}
	return this.toString();
   }
</script>
I find two issue :
1: If disable DFG_JIT , the crash don't happen .
2. If "var time = d.getHours().addNulls() + ":" + d.getMinutes().addNulls();" is changed to "var time = d.getHours() + ":" + d.getMinutes();"
The crash don't happen
It looks like  process happen crash when addNulls  is called
Comment 5 Peng Xinchao 2015-04-18 03:33:40 PDT
It look like  JSC DFG happen error when running  "return "0" + this;"
Because i found that the crash usually happen when "if (this < 10)".
Comment 6 Peng Xinchao 2015-04-18 04:41:03 PDT
The issue 
void SpeculativeJIT::compile(Node* node)
{

}
Comment 7 Peng Xinchao 2015-04-18 04:43:54 PDT
 
void SpeculativeJIT::compile(Node* node)
{
  .. .
    case ToThis: {
        ASSERT(node->child1().useKind() == UntypedUse);
....

}
In here ,  the value of "node->child1().useKind()" is UntypedUse .
I feel very strange 。
why ?
Comment 8 Mark Lam 2015-04-18 06:57:44 PDT
(In reply to comment #7)
>  
> void SpeculativeJIT::compile(Node* node)
> {
>   .. .
>     case ToThis: {
>         ASSERT(node->child1().useKind() == UntypedUse);
> ....
> 
> }
> In here ,  the value of "node->child1().useKind()" is UntypedUse .
> I feel very strange 。
> why ?

Again, r150045 is very old (from 2 years ago).  There has been many enhancements and bug fixes since then.   Have you tried the latest revision (e.g. r182982) to see if the issue is still present?
Comment 9 Peng Xinchao 2015-06-07 23:29:52 PDT
In the newest version (x86_gtk),The crash is still exist .

    case ToThis: {
        printf(" node->child1().useKind() =%d\n",node->child1().useKind());
        ASSERT(node->child1().useKind() == UntypedUse); ---> crash here

TestCase :
<html>
<head>
	<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
	<title>Tutorial Video Widget</title>
	<script>
function onLoad() {
		
			setInterval(function() {
				var d = new Date();
				var time = d.getHours().addNulls() + ":" + d.getMinutes().addNulls();			
				document.write("time="+time);
				document.write("<br>");
			}, 1000);

		}
Number.prototype.addNulls = function () {
			if (this < 100) {
				return "0" + this;
			}
			return this.toString();
		}
</script>
</head>
<body onload="onLoad();">

</body>
</html>
Comment 10 Peng Xinchao 2015-06-07 23:39:56 PDT
Created attachment 254475 [details]
Patch
Comment 11 Filip Pizlo 2015-06-08 08:04:16 PDT
Comment on attachment 254475 [details]
Patch

This is definitely not the right approach. You're just turning off ToThis in the DFG and creating a bunch of dead code. Note that the ToThis handling in DFG is designed to work with UntypedUse - so this patch doesn't make sense.