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

value access to extensible variant tag binds module value instead (name clash) #7563

Closed
vicuna opened this issue Jun 22, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Jun 22, 2017

Original bug ID: 7563
Reporter: fahndrich
Assigned to: @gasche
Status: resolved (set by @gasche on 2017-06-26T15:21:01Z)
Resolution: fixed
Priority: normal
Severity: major
Platform: x86
OS: Linux
OS Version: Linux-gnu Centos
Version: 4.03.0
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: middle end (typedtree to clambda)
Monitored by: @gasche

Bug description

A reference from a module A to B picks up B.C (a module value) instead of B.C an extensible variant tag.

This results in a block with tag 0 containing the module values being passed around instead of an extensible variant value.

Repro is attached.

Steps to reproduce

ocamlopt a_module.ml debug.ml variant.ml repro.ml -o repro
./repro

Prints:
Stored: a Clash
Direct: unknown: tag: 0, Size 3
0: An int 0
1: tag: 247, Size 2
0: <unknown with tag 1001>
1: An int 1
2: tag: 247, Size 2
0: <unknown with tag 1001>
1: An int 1
Fatal error: exception Invalid_argument("compare: functional value")

The first print is getting the right variant value by accessing it via another name.
The second print is getting the wrong module value and I added some debug output to show the contents. As you can see the contents corresponds to the a_module contents (an int (empty list), and 2 closures).

The fatal error is trying to compare the ill-bound variant tag (which is really a module) and causing comparison of closures.

Additional information

Tested on 4.02.3, 4.03.0, 4.04.1, and 4.06.0+trunk and it fails in all cases.

(My category above is just a guess, middle end)

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: @gasche

Thanks for the report! The bug was introduced by module aliases in 4.02. Two ways to make the reproduction case simpler are:

  • A_module does not need to be an external module, it can be declared above (but you need a definition of the form "module Alias = A_module", just "module Alias = struct .. end" would not work). On the other hand, the access to the conflict name needs to be from a different compilation unit.

  • You can use "exception Clash" instead of the extensible datatype (anything that both uses transl_path in bytecomp/translcore.ml and has an uppercase last component would work), which allows to test the code under pre-4.02 version to check that the bug cannot be reproduced.

The following patch fixes the issue, but I am not sure that it is correct overall.

diff --git a/bytecomp/lambda.ml b/bytecomp/lambda.ml
index cca6315..b286772 100644
--- a/bytecomp/lambda.ml
+++ b/bytecomp/lambda.ml
@@ -535,7 +535,7 @@ let rec transl_normal_path = function
(* Translation of value identifiers *)

let transl_path ?(loc=Location.none) env path =

  • transl_normal_path (Env.normalize_path (Some loc) env path)
  • transl_normal_path (Env.normalize_path_prefix (Some loc) env path)

(* Compile a sequence of expressions *)

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: @gasche

Proposed fix (plus a regression testcase) sent at

#1210

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: @yallop

For full dramatic effect, here's a way to turn the bug into a segmentation fault:

$ ocaml
OCaml version 4.04.0

module M = struct

   module X = List
   exception X
 end;;

module M : sig module X = List exception X end

print_endline (Printexc.to_string M.X);;

Segmentation fault

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: @gasche

Reporter: I am about to write a Changes entry, what is the name that should be used for crediting the report? Are you Manuel Fähndrich or Manuel Fahndrich? (Is it correct to assume that the first version is actually the correct one, despite the second one occurring more often on the internet?). If not, what name should be used to credit your work?

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: fahndrich

I'm the same person. Do I have 2 accounts? I dropped the umlaut a long time ago when I went to the US. I only use it in LaTeX papers :-)

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

Comment author: @gasche

Well, googling for "site:caml.inria.fr/mantis fahndirch" only reported issues from way back, submitted also using your first name, so I preferred to ask than just assume. Thanks!

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 26, 2017

Comment author: @gasche

#1210 was merged after a round and review and some changes. It is only in the current trunk, so it will not be part of the next major release 4.05 that is frozen right now, but the following one (4.06).

Thanks for the report!

@vicuna vicuna closed this Jun 26, 2017

@vicuna vicuna added the middle-end label Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.