-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strengthen bound checks for switch indexes #1172
Conversation
LGTM. |
I've removed your commits from master. |
104ae26
to
e3b2db8
Compare
I removed unnecessary commits. You just need to add a test. |
(Sorry for the delayed reply; I will try to reduce the build |
gentle ping |
Sorry, I forgot to report back: I can reproduce the problem, |
I would have preferred to have a small repro but it's unclear when / if we'll get it. The change is obviously good so I decided to merge now |
(Sorry, I did not have the time to work on it. |
…s_of_ocaml-ppx_deriving_json, js_of_ocaml-ppx, js_of_ocaml-lwt and js_of_ocaml-compiler (4.0.0) CHANGES: ## Features/Changes * Compiler: add --target-env flag, for JS runtime specific compilation targets (ocsigen/js_of_ocaml#1160). * Compiler: static evaluation of backend_type (ocsigen/js_of_ocaml#1166) * Compiler: speedup emitting js files (ocsigen/js_of_ocaml#1174) * Compiler: simplify (a | 0) >>> 0 into (a >>> 0) (ocsigen/js_of_ocaml#1177) * Compiler: improve static evaluation of cond (ocsigen/js_of_ocaml#1178) * Compiler: be more consistent dealing with js vs ocaml strings (ocsigen/js_of_ocaml#984) * Compiler: Compiler: add BigInt to provided symbols (fix ocsigen/js_of_ocaml#1168) (ocsigen/js_of_ocaml#1191) * Compiler: use globalThis, drop joo_global_object ocsigen/js_of_ocaml#1193 * Compiler: new -Werror flag to turn wanrings into errors (ocsigen/js_of_ocaml#1222) * Compiler: make the inlining less agressive, reduce size, improve pref (ocsigen/js_of_ocaml#1220) * Compiler: rename internal library js_of_ocaml-compiler.runtime to js_of_ocaml-compiler.runtime-files * Lib: new runtime library to improve compatibility with Brr and gen_js_api * Lib: add messageEvent to Dom_html (ocsigen/js_of_ocaml#1164) * Lib: add PerformanceObserver API (ocsigen/js_of_ocaml#1164) * Lib: add CSSStyleDeclaration.{setProperty, getPropertyValue, getPropertyPriority, removeProperty} (ocsigen/js_of_ocaml#1170) * Lib: make window.{inner,outer}{Width,Height} non-optional * Lib: introduce Js_of_ocaml.Js_error module, deprecate Js_of_ocaml.Js.Error exception. * Lib: add deprecation warning for deprecated code * PPX: json can now be derived for mutable records (ocsigen/js_of_ocaml#1184) * Runtime: use crypto.getRandomValues when available (ocsigen/js_of_ocaml#1209) * Misc: move js_of_ocaml-ocamlbuild out to its own repo * Misc: add support for OCaml 4.14 (ocsigen/js_of_ocaml#1173) ## Bug fixes * Compiler: fix sourcemap warning for empty cma (ocsigen/js_of_ocaml#1169) * Compiler: Strengthen bound checks. (ocsigen/js_of_ocaml#1172) * Compiler: fix `--wrap-with-fun` under node (ocsigen/js_of_ocaml#653, ocsigen/js_of_ocaml#1171) * Compiler: fix parsing of annotaions in js stubs (ocsigen/js_of_ocaml#1212, fix ocsigen/js_of_ocaml#1213) * Ppx: allow apostrophe in lident (fix ocsigen/js_of_ocaml#1183) (ocsigen/js_of_ocaml#1192) * Runtime: fix float parsing in hexadecimal form * Runtime: fix implementation of caml_js_instanceof * Graphics: fix mouse_{x,y} (ocsigen/js_of_ocaml#1206)
This pull request tweaks the bounds checks for
switch indexes in
compiler/lib/eval.ml
in orderto handle negative indexes.
While the change seems sound and in line with
the comment about dead code, it is not entirely
clear to me whether this constitutes a proper fix
or is rather masking a deeper problem.
(I apologize for mistakenly pushing to
master
.)