Skip to content

Bug in generational global roots#607

Merged
gasche merged 1 commit intoocaml:4.03from
jhjourdan:generational_roots_bug
Jun 26, 2016
Merged

Bug in generational global roots#607
gasche merged 1 commit intoocaml:4.03from
jhjourdan:generational_roots_bug

Conversation

@jhjourdan
Copy link
Copy Markdown
Contributor

This commit corrects a bug in the code for generational roots:
[caml_modify_generational_global_root] makes it possible for an old
root to be in the young generation, but [caml_remove_generational_global_root]
did not take this into account.

The bug probably also exists in all recent branches.

[caml_modify_generational_global_root] makes it possible for an old
root to be in the young generation, but [caml_remove_generational_global_root]
did not take this into account.
@xavierleroy
Copy link
Copy Markdown
Contributor

Would you please describe the bug in question?

@jhjourdan
Copy link
Copy Markdown
Contributor Author

Pardon me or the lack of detail:

  • For a failing testcase, you can run the test of my PR without the rest of the patch.
  • The bug itself is that [caml_remove_generational_global_root] assumes that the root being deleted is in [caml_global_roots_old] whenever it is in the major heap, but this is wrong, because of the behavior of [caml_modify_generational_global_root].

@jhjourdan
Copy link
Copy Markdown
Contributor Author

More precisely, a comment in [caml_modify_generational_global_root] says : "It is OK to have a root in roots_young that suddenly points to the old generation". But this is not correct for root removal.

@bobot
Copy link
Copy Markdown
Contributor

bobot commented Jun 9, 2016

Why do you change also caml_modify_generational_global_root?

Le jeu. 9 juin 2016 13:18, Jacques-Henri Jourdan notifications@github.com
a écrit :

More precisely, a comment in [caml_modify_generational_global_root] says :
"It is OK to have a root in roots_young that suddenly points to the old
generation". But this is not correct for root removal.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#607 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAGRwRVtD8K4mrm8JKDqwD6Mw3xRaspGks5qJ_aTgaJpZM4IwcrA
.

@jhjourdan
Copy link
Copy Markdown
Contributor Author

This part of caml_modify_generational_global_root is executed whenever the root becomes unboxed. In this case, we want to delete the root. Hence, the code needs to do the same thing as remove.

@damiendoligez damiendoligez added this to the 4.04 milestone Jun 14, 2016
@gasche gasche merged commit 2449d2f into ocaml:4.03 Jun 26, 2016
@gasche
Copy link
Copy Markdown
Member

gasche commented Jun 26, 2016

I cliked the tempting "merge" button but the change went in the 4.03 branch -- thanks to @hannesm for the notice. I cherry-picked in trunk ( 69d0d23 ).

@damiendoligez , should we put bugfixes in 4.03? Is there a 4.03.1 planned?

@jhjourdan
Copy link
Copy Markdown
Contributor Author

Sorry, 4.03 was the branch I was working on, so I did a patch on it, thinking you would backport it anyway... But indeed, if there is no 4.03.1 planned, this is silly.

@damiendoligez
Copy link
Copy Markdown
Member

I will merge the 4.03 branch into trunk very soon and close it. There will not be a 4.03.1 release.

stedolan added a commit to stedolan/ocaml that referenced this pull request May 24, 2022
lukemaurer pushed a commit to lukemaurer/ocaml that referenced this pull request Jul 19, 2022
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
ce76e02 flambda-backend: Bugfix for type_application (ocaml#746)
44f3afb flambda-backend: PR580 for main branch (ocaml#743)
b851eaa flambda-backend: Backport first part of ocaml/ocaml PR10498 (ocaml#737)
fafb4bd flambda-backend: Fix return mode for eta-expanded function in type_argument (ocaml#735)
c31f6c3 flambda-backend: Fix treatment of functions called [@nontail] (ocaml#725)
847781e flambda-backend: Fix build_upstream post-PR703 (ocaml#712)
bfcbbf8 flambda-backend: Extend Pblock value kind to handle variants (ocaml#703)
b2cab95 flambda-backend: Merge ocaml-jst
a6d6e0e flambda-backend: Merge ocaml-jst
88a4f63 flambda-backend: Use Pmakearray for immutable arrays (ocaml#699)
eeaa44b flambda-backend: Install an ocamldoc binary (ocaml#695)
48d322b flambda-backend: Ensure that GC is not invoked from bounds check failures (ocaml#681)
4370fa1 flambda-backend: Review changes of term directory (ocaml#602)
65a4566 flambda-backend: Add code coverage using bisect_ppx (ocaml#352)
63ab65f flambda-backend: Bugfix for primitive inclusion (ocaml#662)
7e3e0c8 flambda-backend: Fix inclusion checks for primitives (ocaml#661)
96c68f9 flambda-backend: Speed up linking by changing cmxa format (ocaml#607)
1829150 flambda-backend: Bugfix for Translmod.all_idents (ocaml#659)

git-subtree-dir: ocaml
git-subtree-split: ce76e02
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
* Turn hash into an optional parameter

* Define helper function with_hash, allowing to deal with
  optional hash parameter in applicable functions
* Add hash as optional parameter in functions:
  + package,
  + package_with_version
  + package_doc
* Remove corresponding with_{hash|univ} functions (in order):
  + package_with_univ
  + package_with_hash_with_version
  + package_doc_with_hash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants