ClojureScript

Hashing regression with JavaScriptCore

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

With https://github.com/clojure/clojurescript/commit/6ed5857aa49f28dc81390e3522e85b497be0f9de Weasel (https://github.com/tomjakubowski/weasel) functionality regressed for JavaScriptCore/Safari (but not with Firefox or Chrome), where the result appears to the end user as a Weasel hang when issuing a browser REPL command.

At its core, it appears to be related to a failure to parse / read a Weasel command of the form {:op :eval-js, :code "cljs.core.pr_s ... <omitted for brevity>"} within the browser, where the :op :eval-js keyword/value pair is not parsed, resulting in a subsequent multimethod dispatching on :op in Weasel's process-message multimethod to derail.

More detail is in https://github.com/james-henderson/simple-brepl/issues/4, but my plan is to attempt to formulate a minimal setup to reproduce the failure.

Activity

Hide
Mike Fikes added a comment - - edited

Perhaps a minimal reproduction of the failure is achieved by invoking the cljs reader directly in JavaScript as described below:

Within the Safari JavaScript REPL, evaluating the following

> cljs.reader.read_string.call(null, "{:op :eval-js :code :x}");

results in a JavaScript object representing the map. Prior to the hashing commit referenced in this ticket's description, this map representation has 4 elements in its root arr (representing the 2 key/val pairs), where element 0 is "op", with a _hash value of 1013907795, which matches the :op keyword expression that is emitted in various places in my JavaScript output (new cljs.core.Keyword(null, "op", "op", 1013907795)).

After the hashing commit, evaluating the same at the JavaScript REPL, the resulting map representation is a bit "wonky" in Safari with respect to hash values (but is similar to that described in the previous paragraph, in Firefox). By "wonky", it's root arr has 2 elements, the 1st being null, and the 2nd containing an arr of length 4 (which appears to be the 2 key/val pairs, simply a bit lower in the trie), but where the hash is 1013904242 for both the item representing the "op" keyword and for the item representing the "code" keyword, and since this hash differs from that for the :op keyword expression (new cljs.core.Keyword(null, "op", "op", -1882987955)), the attempt to get :op from the map fails. (Note that in the Firefox debugger, the element associated with "op" has a hash of 2411979341, which actually matches -1882987955 (the first being an unsigned and the 2nd being 2's complement representation of the same 32-bit hex value).

Show
Mike Fikes added a comment - - edited Perhaps a minimal reproduction of the failure is achieved by invoking the cljs reader directly in JavaScript as described below: Within the Safari JavaScript REPL, evaluating the following > cljs.reader.read_string.call(null, "{:op :eval-js :code :x}"); results in a JavaScript object representing the map. Prior to the hashing commit referenced in this ticket's description, this map representation has 4 elements in its root arr (representing the 2 key/val pairs), where element 0 is "op", with a _hash value of 1013907795, which matches the :op keyword expression that is emitted in various places in my JavaScript output (new cljs.core.Keyword(null, "op", "op", 1013907795)). After the hashing commit, evaluating the same at the JavaScript REPL, the resulting map representation is a bit "wonky" in Safari with respect to hash values (but is similar to that described in the previous paragraph, in Firefox). By "wonky", it's root arr has 2 elements, the 1st being null, and the 2nd containing an arr of length 4 (which appears to be the 2 key/val pairs, simply a bit lower in the trie), but where the hash is 1013904242 for both the item representing the "op" keyword and for the item representing the "code" keyword, and since this hash differs from that for the :op keyword expression (new cljs.core.Keyword(null, "op", "op", -1882987955)), the attempt to get :op from the map fails. (Note that in the Firefox debugger, the element associated with "op" has a hash of 2411979341, which actually matches -1882987955 (the first being an unsigned and the 2nd being 2's complement representation of the same 32-bit hex value).
Hide
Mike Fikes added a comment -

An equivalent, but perhaps simpler reduction of the bug:

(ns foo.bar
(:require [cljs.reader :as reader]))

(println "Expecting :js-eval here:" (:op (reader/read-string "{:op :js-eval :code :x}")))

With 0.0-2261 this will print

Expecting :js-eval here: nil

but with 0.0-2234 this will print

Expecting :js-eval here: :js-eval

if you run this in the shipping version of JavaScriptCore (Safari or iOS).

Note that this bug does not occur if you use a WebKit nightly. Thus, automated integration / unit tests for this against WebKit nightly fail to detect the problem.

Show
Mike Fikes added a comment - An equivalent, but perhaps simpler reduction of the bug: (ns foo.bar (:require [cljs.reader :as reader])) (println "Expecting :js-eval here:" (:op (reader/read-string "{:op :js-eval :code :x}"))) With 0.0-2261 this will print Expecting :js-eval here: nil but with 0.0-2234 this will print Expecting :js-eval here: :js-eval if you run this in the shipping version of JavaScriptCore (Safari or iOS). Note that this bug does not occur if you use a WebKit nightly. Thus, automated integration / unit tests for this against WebKit nightly fail to detect the problem.
Hide
Mike Fikes added a comment -

I think this is valid simpler reduction, focusing on the hash of a read keyword (these numbers reflect those 2 comments back):

(println (hash :op))
;=> -1882987955

(println (hash (let [r (reader/push-back-reader "op ")]
(reader/read-keyword r nil))))
;=> 1013904242

Show
Mike Fikes added a comment - I think this is valid simpler reduction, focusing on the hash of a read keyword (these numbers reflect those 2 comments back): (println (hash :op)) ;=> -1882987955 (println (hash (let [r (reader/push-back-reader "op ")] (reader/read-keyword r nil)))) ;=> 1013904242
Hide
Mike Fikes added a comment -

A simpler reduction, independent of the reader code:

(hash :op)
;=> -1882987955

(hash (keyword "op"))
;=> 1013904242

Show
Mike Fikes added a comment - A simpler reduction, independent of the reader code: (hash :op) ;=> -1882987955 (hash (keyword "op")) ;=> 1013904242
Hide
Mike Fikes added a comment -

Perhaps it is related to hash cacheing rather than hash computation:

(hash (keyword "op"))
;=> 1013904242

(hash (keyword "code"))
;=> 1013904242

(hash (keyword "abc"))
;=> 1013904242

Show
Mike Fikes added a comment - Perhaps it is related to hash cacheing rather than hash computation: (hash (keyword "op")) ;=> 1013904242 (hash (keyword "code")) ;=> 1013904242 (hash (keyword "abc")) ;=> 1013904242
Hide
David Nolen added a comment -

All released versions of Safari 7 have a broken implementation of Math.imul, fall back to non-native imul implemented now in master https://github.com/clojure/clojurescript/commit/e92e8064813ed9a74c6dcf5bfd3adf5b85df1aea

Show
David Nolen added a comment - All released versions of Safari 7 have a broken implementation of Math.imul, fall back to non-native imul implemented now in master https://github.com/clojure/clojurescript/commit/e92e8064813ed9a74c6dcf5bfd3adf5b85df1aea

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: