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
typeopt.ml unsafely assumes all modules called Bigarray are the same #7552
Comments
Comment author: @gasche This comes from bytecomp/typeopt.ml: let bigarray_decode_type env ty tbl dfl = In trunk (as opposed to 4.05) "CamlinternalBigarray" is used instead, but the test is still purely syntactic so the same trick would work. |
Comment author: @gasche I don't know how to fix this without adding the kind types to typing/Predef, which (if I understand correctly) are the only ones that the compiler can reason about using absolute (non-shadowable) names. |
Comment author: @alainfrisch This is perhaps naive, but couldn't we check [Ident.global mod_id]? It would still not protect against shadowing the global bigarray.cmi (which requires linking without the stdlib), but that would reduce the problem quite a bit. |
Comment author: @lpw25 I think Gabriel's suggestion is the correct solution: the compiler should really only be reasoning about abstract types it defined. I agree that Alain's suggestion would make it less of a problem, and we should probably do that in the short term if no one has time to do the right thing at the moment. |
Comment author: @alainfrisch The compiler also does name-based lookups on CamlinternalOO and CamlinternalMod during code generation. Linking against a "non-standard" stdlib is already unsafe for this reason, I believe. |
Comment author: @xavierleroy In 4.06 and up the example works fine because the crucial type definitions are now in CamlinternalBigarray. That name is enough protection against accidental shadowing, in my opinion. Malicious users can still shadown the various Camlinternal* modules with their own concotions, but there are much easier ways to cause an OCaml program to segfault. As to "the right thing" being to predefine more things within the compiler,I don't think it's sustainable. It might work here because only types are involved but would not work for the other Camlinternal* modules that also define functions and values. I'd be more interested in lightweight ways to "bless" a module as being the standard library module the compiler expects. |
Original bug ID: 7552
Reporter: @lpw25
Status: resolved (set by @xavierleroy on 2017-09-30T16:06:49Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.04.1
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: middle end (typedtree to clambda)
Monitored by: @gasche
Bug description
The following program will segfault:
module Alias = struct module Bigarray = Bigarray end
module Bigarray : sig
end = struct
end
let x = Bigarray.x
open Alias
let f = x.{0}
let () = Printf.printf "%f\n%!" f
The issue is that typeopt.ml assumes that the [int_elt] type from the [Bigarray] submodule is the same as the normal [Bigarray.int_elt] and so incorrectly optimises the array access.
The text was updated successfully, but these errors were encountered: