Skip to content
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

segfault with shadowing/include #6435

Closed
vicuna opened this issue May 20, 2014 · 4 comments
Closed

segfault with shadowing/include #6435

vicuna opened this issue May 20, 2014 · 4 comments

Comments

@vicuna
Copy link

vicuna commented May 20, 2014

Original bug ID: 6435
Reporter: @diml
Assigned to: @garrigue
Status: closed (set by @xavierleroy on 2017-02-16T14:16:23Z)
Resolution: fixed
Priority: high
Severity: crash
Version: 4.02.0+dev
Target version: 4.02.0+dev
Fixed in version: 4.02.0+dev
Category: back end (clambda to assembly)
Related to: #7067
Monitored by: @gasche @jmeber @hcarty

Bug description

The following program segfault with trunk:

-----[ foo.ml ]-----

module M = struct
type t = string

let x = 0
let x = 1

module Set = Set.Make(String)
end

include M

-----[ main.ml ]-----

module F (M : sig
type t
module Set : Set.S with type elt = t
end) =
struct
let test set = Printf.printf "%d\n" (M.Set.cardinal set)
end

module M = F (Foo)

let () = M.test (Foo.M.Set.singleton "42")


compile with: ocamlbuild main.native

It is easy to see what is wrong by looking at the lambda code:

The field for [Foo.M] is compiled as:

(setfield_imm 0 (global Foo!)
(makeblock 0
(field 1 M/1117) ; let x = 1
(field 2 M/1117) ; module Set = Set.Make(String)
))

And [Main.M]:

(M/1165 =
(apply (field 0 (global Main!))
(let (let/1316 =a (global Foo!))
(makeblock 0
(field 2 (field 0 let/1316)) ; [Foo.M].(2) while [Foo.M] has only 2 fields!
))))

The (field 2 ) seems to come from (field 2 M/...) of [Foo]. Adding more [let x = ...] in foo.ml increases the offset.

@vicuna
Copy link
Author

vicuna commented May 20, 2014

Comment author: @mshinwell

Just for the record: I think this one took more than ten full days of debugging to find!

I've confirmed that this bug was introduced by the original module alias patches.
Another symptom may be "Fatal error: out of memory" caused by heap corruption.

@vicuna
Copy link
Author

vicuna commented May 21, 2014

Comment author: @garrigue

Fixed in 4.02, at revision 14896.

The problem was that Typemod.simplify_signature could remove some fields in submodules, without updating the corresponding alias paths (the offsets are contained in paths).

Rather than do the tricky job of updating paths, I settled for the cleaner solution of simplifying signatures immediately: introduce a coercion at the definition point if a module contains a shadowed field. As a result simplify_signature is no longer recursive.

@vicuna
Copy link
Author

vicuna commented Nov 30, 2015

Comment author: @alainfrisch

I settled for the cleaner solution of simplifying signatures immediately

This might be the cause for a serious degradation of compile time, see #7067.

@vicuna
Copy link
Author

vicuna commented Nov 30, 2015

Comment author: @mshinwell

I think we should check this with flambda (see #7067) before spending time fixing this any further...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants