Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005772OCamlOCaml runtime systempublic2012-10-04 11:312012-10-05 18:05
Reporterccpasteur 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSMAC OS XOS Version10.6.8
Product Version4.00.0 
Target VersionFixed in Version4.00.2+dev 
Summary0005772: Bug in marshaling of recursive functions
DescriptionAfter declaring with two recursive functions (and a few other functions), marshaling fails with the error:
Fatal error: exception Invalid_argument("output_value: abstract value (outside heap)"). It does not fail if we change the order of declaration of the two recursive functions.


Steps To ReproduceCompile the file with ocamlc or ocamlopt and run it.
Additional InformationThe module is declared by:
module P = struct
  let rec test_one q x = x > 3
  and test_list q = List.for_all (test_one q) q

  let g () = ()

  let f q =
    if test_list q then
      g ()
end

Marshalling f fails. The weird thing is that it does not fail if we change the order of declarations of the recursive functions. It also does not fail if we remove the call to g in f. Marshalling test_list works.

The bug happens with bytecode and native code. It does not happen in OCaml 3.12.1 .
TagsNo tags attached.
Attached Files? file icon test.ml [^] (232 bytes) 2012-10-04 11:31 [Show Content]
? file icon patch [^] (489 bytes) 2012-10-04 20:51 [Show Content]
patch file icon marshal_test.patch [^] (1,826 bytes) 2012-10-05 00:02 [Show Content]

- Relationships

-  Notes
(0008197)
gasche (developer)
2012-10-04 14:38

Not-so-surprisingly, I have bisected back the bug to the following two commits:

r12248
PR#5318: Reverting last un-marshaler changes, to rework the control flow of mainloop and provide better solution for eliminating recursion

r12247
PR#5318: Non-recursive version of extern_rec and intern_rec, to allow marshaling and un-marshaling of deeper data structures
(0008201)
jacques-henri.jourdan (manager)
2012-10-04 20:54

I submitted a patch that fixes it !
(0008202)
gasche (developer)
2012-10-04 23:58

Indeed I can confirm that Jacques-Henri's patch fixes the issue. I have uploaded ccpasteur's initial example as patch to modify the existing "lib-marshal" testsuite, that makes apparent the issue and its solution, while allowing to check that the fix didn't break any of the lib-marshal tests.
(0008204)
meyer (developer)
2012-10-05 04:56

Gabriel: To be honest and at the same time precise, the commits you pointed out are not a full story;) To my satisfaction.

Jacques-Henri: Thanks for the patch. I'll review the patch once more tomorrow and apply if it's correct.

Personally, I think that your patch should be included in the current patch release. (if proven correct).
(0008206)
gasche (developer)
2012-10-05 07:34

Indeed, I did not intend to mean that the patch were bad, just that a "git bisect" showed that the error appeared precisely there. I did review the first one at the time (obviously I missed this problem), and I think they're kind of cool -- defunctionalization in the real world!

Regarding the current patch release, Damien will decide (because that would be work for him in the end), but indeed I think that would be a good idea.
(0008208)
xleroy (administrator)
2012-10-05 09:27

Yes, Jacques-Henri's patch looks right to me. Will see what our almighty release manager thinks about inclusion in 4.01.

As to blaming, I probably share some of the guilt for missing this issue while reviewing the new marshaler implementation.
(0008210)
xleroy (administrator)
2012-10-05 18:05

Applied the patches by jacques-henri.jourdan and gasche, in 4.00 bugfix branch (commit 12991) and in trunk (commit 12992).

- Issue History
Date Modified Username Field Change
2012-10-04 11:31 ccpasteur New Issue
2012-10-04 11:31 ccpasteur File Added: test.ml
2012-10-04 14:38 gasche Note Added: 0008197
2012-10-04 20:51 jacques-henri.jourdan File Added: patch
2012-10-04 20:54 jacques-henri.jourdan Note Added: 0008200
2012-10-04 20:54 jacques-henri.jourdan Note Added: 0008201
2012-10-04 20:54 jacques-henri.jourdan Note Deleted: 0008200
2012-10-04 23:56 gasche File Added: marshal_test.patch
2012-10-04 23:58 gasche Note Added: 0008202
2012-10-04 23:59 gasche Status new => confirmed
2012-10-05 00:00 gasche File Deleted: marshal_test.patch
2012-10-05 00:02 gasche File Added: marshal_test.patch
2012-10-05 04:56 meyer Note Added: 0008204
2012-10-05 07:34 gasche Note Added: 0008206
2012-10-05 09:27 xleroy Note Added: 0008208
2012-10-05 18:05 xleroy Note Added: 0008210
2012-10-05 18:05 xleroy Status confirmed => resolved
2012-10-05 18:05 xleroy Resolution open => fixed
2012-10-05 18:05 xleroy Fixed in Version => 4.00.2+dev


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker