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

Printing lazy value segfaults ocamldebug #6684

Closed
vicuna opened this issue Nov 29, 2014 · 6 comments
Closed

Printing lazy value segfaults ocamldebug #6684

vicuna opened this issue Nov 29, 2014 · 6 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Nov 29, 2014

Original bug ID: 6684
Reporter: @gasche
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:37:08Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.1
Target version: 4.03.0+dev / +beta1
Category: tools (ocaml{lex,yacc,dep,debug,...})
Tags: patch
Has duplicate: #5836
Related to: #6669

Bug description

Using the "print foo" ocamldebug command, to print the value of the variable "foo" in the current context, segfaults when "foo" is a lazy value that has already been forced.

(This issue has been reported to me by the Coq people, which use ocamldebug as part of their debugging workflow.)

Steps to reproduce

cat > test.ml <<EOF
let x = lazy (1 + 2)
let () = ignore (Lazy.force x)
let () = for i = 0 to 10 do () done
EOF
ocamlc -g -o test test.ml
ocamldebug ./test <<EOF
run
prev 2
print x
EOF

Additional information

Observed behavior under any OCaml version >= 3.12.1:

OCaml Debugger version 4.03.0+dev5-2014-10-15

(ocd) Loading program... done.
Time: 46
Program exit.
(ocd) Time: 30 - pc: 31880 - module Test
3 let () = for i = 0 to 10 do <|b|>() done
(ocd) Fatal error: exception End_of_file
Fatal error: exception End_of_file
Segmentation fault (core dumped)

File attachments

@vicuna
Copy link
Author

vicuna commented Nov 29, 2014

Comment author: @gasche

I investigated the bug and found two different sources of issue:

  • The pretty-printing code for lazy values uses Lazy.is_val and Lazy.force, which internally rely on the Obj module. This is wrong because the pretty-printing code is wrapped under a functor that abstracts over the implementation of Obj, and the debugger instantiates it with a different implementation that talks over the debugging socket. Fixing this makes the segfault go away if the lazy value is a string.

  • The alternate implementation of Obj used by the debugger does not properly compute the tag (Obj.tag) of unboxed integers, which causes a (different) segfault when the lazy value is an integer.

I'll upload patches which seem to fix those issues -- comments appreciated.

@vicuna
Copy link
Author

vicuna commented Nov 29, 2014

Comment author: @gasche

Patch 0003 concerns the "is_block" check in the debugger/debugcomm.ml implementation of "tag".

Patch 0001 and 0002 remove the uses of Obj instead of the functor argument O in the code of toplevel/genprintval.ml. Patch 0001 fixes the printing of lazy value, and Patch 0002 fixes the cycle-detection logic.

I verified through testing that 0001 and 0003 fix segfaults. I do not have a reproduction case for an issue related to 0002.

@vicuna
Copy link
Author

vicuna commented Nov 29, 2014

Comment author: @gasche

(The patches are also available on github:
https://github.com/gasche/ocaml/compare/ocamldebug-lazy
)

@vicuna
Copy link
Author

vicuna commented Dec 5, 2014

Comment author: @damiendoligez

In the first patch, you are still using Obj.lazy_tag and Obj.forward_tag. I know they are the same in debugger and debuggee as soon as the system is bootstrapped, but it would be cleaner to add them to the OBJ signature and use O.lazy_tag and O.forward_tag instead.

For the second patch, presumably a remote value is represented by an int iff it is an int, so the old code does work. But your way is cleaner.

For the third patch, you could just add one clause at the beginning of the function's pattern-matching:
| x when not (is_block x) -> Obj.int_tag
but it's mostly a matter of style.

@vicuna
Copy link
Author

vicuna commented Dec 6, 2014

Comment author: @gasche

I'm not sure about {O,Obj}.{lazy,forward}_tag. Does it mean that we should add the 15 foo_tag values from Obj into the OBJ signature? Or is the idea to add only those on which the implementation actually relies?

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @gasche

I made changes as per Leo's comments, and merged mostly as-is in trunk.

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