Now, there are 35 tests failing into Debug builds: https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/1926/steps/jscore-test/logs/stdio
Created attachment 357768 [details] Patch
Comment on attachment 357768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357768&action=review > Source/JavaScriptCore/ChangeLog:10 > + Current rule is considering that both operands of ValueAdd have nodes, > + but this is not quite true. In such case, we are checking the > + existence of both nodes before using m_node->binaryUseKind. I don't understand: when would ValueAdd ever not have children nodes? Also, please provide a test case. The test will help illustrate the issue.
Comment on attachment 357768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357768&action=review >> Source/JavaScriptCore/ChangeLog:10 >> + existence of both nodes before using m_node->binaryUseKind. > > I don't understand: when would ValueAdd ever not have children nodes? Also, please provide a test case. The test will help illustrate the issue. Oh, I see there are already failing tests. I'm still surprised that ValueAdd has edges that don't have nodes. Perhaps there's something I don't know.
Comment on attachment 357768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357768&action=review >>> Source/JavaScriptCore/ChangeLog:10 >>> + existence of both nodes before using m_node->binaryUseKind. >> >> I don't understand: when would ValueAdd ever not have children nodes? Also, please provide a test case. The test will help illustrate the issue. > > Oh, I see there are already failing tests. I'm still surprised that ValueAdd has edges that don't have nodes. Perhaps there's something I don't know. Yeah, I don't think the current situation is right. ValueAdd must have two children. If it happens, it's a bug. We should fix that bug instead of checking `child1().node() && child2().node()`.
Thx for the review. (In reply to Yusuke Suzuki from comment #4) > Comment on attachment 357768 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=357768&action=review > > >>> Source/JavaScriptCore/ChangeLog:10 > >>> + existence of both nodes before using m_node->binaryUseKind. > >> > >> I don't understand: when would ValueAdd ever not have children nodes? Also, please provide a test case. The test will help illustrate the issue. > > > > Oh, I see there are already failing tests. I'm still surprised that ValueAdd has edges that don't have nodes. Perhaps there's something I don't know. > > Yeah, I don't think the current situation is right. ValueAdd must have two > children. > If it happens, it's a bug. We should fix that bug instead of checking > `child1().node() && child2().node()`. Makes sense. The bug happens when there are 2 constant Strings into ValueAdd and we perform the folding. When this happened we should not execute "m_node->binaryUseKind()". I'm closing this to fix into original Bug https://bugs.webkit.org/show_bug.cgi?id=192723 since we rolled this out.
*** This bug has been marked as a duplicate of bug 192723 ***