Bug 119408 - Incorrect behavior from non-obvious [[ToPrimitive]] conversions for MakeRope
Summary: Incorrect behavior from non-obvious [[ToPrimitive]] conversions for MakeRope
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-01 13:47 PDT by Oliver Hunt
Modified: 2013-08-01 15:17 PDT (History)
1 user (show)

See Also:


Attachments
Testcase (318 bytes, text/plain)
2013-08-01 13:47 PDT, Oliver Hunt
no flags Details
Patch (5.58 KB, patch)
2013-08-01 14:53 PDT, Oliver Hunt
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2013-08-01 13:47:58 PDT
Created attachment 207948 [details]
Testcase

We end up with bogus behavior from the DFG when attempting to perform [[ToPrimitive]] on values with exciting defaultValue behavior.

function f(a,b,c,d,e) { return "1"+a+b+c+d+e+"6"; }
var a=new String("2")
var b = 5;
var e = false;
var k={valueOf:function(){return 4;}, toString:function(){return {}}}
s=String

var h =""; for (var i = 0; i < 200; i++) h+=f(a,new s(3),k, b, e)
h = h.replace(/12345false6/g, "")
print("Passed: " + (h.length == 0));


The function f results in:
f#BJa4XW:[0x7fee8a5022f0->0x14a7cfe70, BaselineFunctionCall]: 43 m_instructions; 344 bytes; 6 parameter(s); 7 callee register(s); 0 variable(s)[   0] enter
[   1] mov		 r0, String (identifier): 1(@k0)
[   4] mov		 r1, r-8
[   7] to_primitive	 r1, r1    rare case: 66
[  10] mov		 r2, r-9
[  13] to_primitive	 r2, r2    rare case: 66
[  16] mov		 r3, r-10
[  19] to_primitive	 r3, r3    rare case: 66
[  22] mov		 r4, r-11
[  25] to_primitive	 r4, r4
[  28] mov		 r5, r-12
[  31] to_primitive	 r5, r5
[  34] mov		 r6, String (identifier): 6(@k1)
[  37] strcat		 r0, r0, 7
[  41] ret		 r0

Constants:
   k0 = String (identifier): 1
   k1 = String (identifier): 6


Which eventually produces:
Block #0 (bc#0):  (OSR target)
  Predecessors:
  Dominated by: #0
  Dominates: #0
  vars before: arg5:(Top, TOP, TOP, TOP) arg4:(Int) arg3:(Top, TOP, TOP, TOP) arg2:(Cell, TOP, TOP, TOP) arg1:(Cell, TOP, TOP, TOP) arg0:(Top, TOP, TOP, TOP)
  var links: arg5:@5 arg4:@4 arg3:@3 arg2:@2 arg1:@1 arg0:@0
   0:  skipped  < 0:->	SetArgument(arg0(a), W:SideState, bc#0)
   1:           < 2:->	SetArgument(, arg1(B<StringObject>), W:SideState, bc#0)  predicting Stringobject
   2:           < 2:->	SetArgument(, arg2(C<StringObject>), W:SideState, bc#0)  predicting Stringobject
   3:           < 2:->	SetArgument(, arg3(D~<Final>), W:SideState, bc#0)  predicting Final
   4:           < 2:->	SetArgument(, arg4(E<Int32>), W:SideState, bc#0)  predicting Int
   5:           < 2:->	SetArgument(, arg5(F~<Boolean>), W:SideState, bc#0)  predicting Bool
   6:           < 2:0>	JSConstant(JS|PureNum|UseAsOther, $0 = String (identifier): 1, bc#1)
   7:  skipped  < 0:->	MovHint(@6<StringIdent>, r0(G~<StringIdent>), W:SideState, bc#1)
   8:           < 1:1>	GetLocal(@1, JS|PureNum|UseAsOther, arg1(B<StringObject>), R:Variables(-8), bc#4)  predicting Stringobject
   9:  skipped  < 0:->	MovHint(@8<StringObject>, r1(H~<StringObject>), W:SideState, bc#4)
  10:           <!2:1>	ToString(Check:StringObject:@8<StringObject>, JS|MustGen|Clobbers|PureNum|UseAsOther|CanExit, R:JSCell_structure, bc#7)
  11:  skipped  < 0:->	MovHint(@10<String>, r1(I~<String>), W:SideState, bc#7)
  12:           < 1:2>	GetLocal(@2, JS|PureNum|UseAsOther, arg2(C<StringObject>), R:Variables(-9), bc#10)  predicting Stringobject
  13:  skipped  < 0:->	MovHint(@12<StringObject>, r2(J~<StringObject>), W:SideState, bc#10)
  14:           <!2:2>	ToString(Check:StringObject:@12<StringObject>, JS|MustGen|Clobbers|PureNum|UseAsOther|CanExit, R:JSCell_structure, bc#13)
  15:  skipped  < 0:->	MovHint(@14<String>, r2(K~<String>), W:SideState, bc#13)
  16:           < 1:3>	GetLocal(@3, JS|PureNum|UseAsOther, arg3(D~<Final>), R:Variables(-10), bc#16)  predicting Final
  17:  skipped  < 0:->	MovHint(@16<Final>, r3(L~<Final>), W:SideState, bc#16)
  18:           <!2:3>	ToPrimitive(@16<Final>, JS|MustGen|Clobbers|PureNum|UseAsOther, R:World, W:World, bc#19)
  19:  skipped  < 0:->	MovHint(@18<String>, r3(M~<String>), W:SideState, bc#19)
  20:           < 2:4>	GetLocal(@4, JS|PureNum|UseAsOther, arg4(E<Int32>), R:Variables(-11), bc#22)  predicting Int
  21:  skipped  < 0:->	MovHint(@20<Int32>, r4(N~<Int32>), W:SideState, bc#22)
  22:           <!0:->	Phantom(Int32:@20<Int32>, MustGen, bc#25)
  23:  skipped  < 0:->	MovHint(@20<Int32>, r4(O~<Int32>), W:SideState, bc#25)
  24:           < 1:5>	GetLocal(@5, JS|PureNum|UseAsOther, arg5(F~<Boolean>), R:Variables(-12), bc#28)  predicting Bool
  25:  skipped  < 0:->	MovHint(@24<Boolean>, r5(P~<Boolean>), W:SideState, bc#28)
  26:           <!1:5>	ToPrimitive(@24<Boolean>, JS|MustGen|Clobbers|PureNum|UseAsOther, R:World, W:World, bc#31)
  27:  skipped  < 0:->	MovHint(@26<Boolean>, r5(Q~<Boolean>), W:SideState, bc#31)
  28:           < 2:6>	JSConstant(JS|PureNum|UseAsOther, $1 = String (identifier): 6, bc#34)
  29:  skipped  < 0:->	MovHint(@28<StringIdent>, r6(R~<StringIdent>), W:SideState, bc#34)
  30:           <!0:->	Phantom(String:@6<StringIdent>, MustGen, bc#37)
  31:           <!0:->	Phantom(String:@10<String>, MustGen, bc#37)
  32:           <!0:->	Phantom(String:@14<String>, MustGen, bc#37)
  33:           < 1:2>	MakeRope(KnownString:@6<StringIdent>, KnownString:@10<String>, KnownString:@14<String>, JS|UseAsOther, R:GCState, W:GCState, bc#37)
  34:           <!0:->	Phantom(Check:String:@18<String>, MustGen|CanExit, bc#37)
  35:           <!1:4>	ToString(@20<Int32>, JS|MustGen|MightClobber|UseAsOther, R:World, W:World, bc#37)
  36:           < 1:4>	MakeRope(KnownString:@33<String>, KnownString:@18<String>, KnownString:@35<String>, JS|UseAsOther, R:GCState, W:GCState, bc#37)
  37:           <!1:5>	ToString(@26<Boolean>, JS|MustGen|MightClobber|UseAsOther, R:World, W:World, bc#37)
  38:           <!0:->	Phantom(String:@28<StringIdent>, MustGen, bc#37)
  39:           < 1:6>	MakeRope(KnownString:@36<String>, KnownString:@37<String>, KnownString:@28<StringIdent>, JS|UseAsOther, R:GCState, W:GCState, bc#37)
  40:  skipped  < 0:->	MovHint(@39<String>, r0(S~<String>), W:SideState, bc#37)
  41:           <!0:->	Flush(@5, MustGen, arg5(F~<Boolean>), W:SideState, bc#41)  predicting Bool
  42:           <!0:->	Flush(@4, MustGen, arg4(E<Int32>), W:SideState, bc#41)  predicting Int
  43:           <!0:->	Flush(@3, MustGen, arg3(D~<Final>), W:SideState, bc#41)  predicting Final
  44:           <!0:->	Flush(@2, MustGen, arg2(C<StringObject>), W:SideState, bc#41)  predicting Stringobject
  45:           <!0:->	Flush(@1, MustGen, arg1(B<StringObject>), W:SideState, bc#41)  predicting Stringobject
  46:           <!0:->	Return(@39<String>, MustGen, W:SideState, bc#41)
  vars after: 
  var links: arg5:@24<Boolean> arg4:@20<Int32> arg3:@16<Final> arg2:@12<StringObject> arg1:@8<StringObject> arg0:@0 r0:@40 r1:@11 r2:@15 r3:@19 r4:@23 r5:@27 r6:@29



Where the function f eventually produces:
1undefinedundefined45false6
Comment 1 Oliver Hunt 2013-08-01 13:48:18 PDT
<rdar://problem/14580940>
Comment 2 Oliver Hunt 2013-08-01 14:53:44 PDT
Created attachment 207955 [details]
Patch
Comment 3 Oliver Hunt 2013-08-01 14:57:15 PDT
Actually <rdar://14625273>
Comment 4 Oliver Hunt 2013-08-01 15:17:52 PDT
Committed r153615: <http://trac.webkit.org/changeset/153615>