Skip to content

Add a test for frametable setup in natdynlinked libraries#983

Merged
mshinwell merged 3 commits intooxcaml:mainfrom
lukemaurer:test-frametable
Nov 24, 2022
Merged

Add a test for frametable setup in natdynlinked libraries#983
mshinwell merged 3 commits intooxcaml:mainfrom
lukemaurer:test-frametable

Conversation

@lukemaurer
Copy link
Copy Markdown
Contributor

@lukemaurer lukemaurer commented Nov 24, 2022

Previously, we could miss that caml_shared_startup wasn't being invoked (see PR #980), causing memory corruption in rare cases due to the frametable missing information on caml_curry_* functions.

@lukemaurer
Add a test for frametable setup in natdynlinked libraries
9f88404 
Previously, we could miss that `caml_shared_startup` wasn't being invoked (see
PR oxcaml#980), causing memory corruption in rare cases due to the frametable missing
information on `caml_curry_*` functions.
@lukemaurer
Promote test output
d3e2a37
@lukemaurer
Be more robust in forcing a non-tail call
3de8052
@mshinwell mshinwell merged commit c458644 into oxcaml:main Nov 24, 2022
gretay-js pushed a commit that referenced this pull request Nov 24, 2022
@lukemaurer @gretay-js
Add a test for frametable setup in natdynlinked libraries (#983)
79a99ec
@stedolan
Copy link
Copy Markdown
Contributor

stedolan commented Nov 28, 2022

It looks like this wasn't caught earlier because caml_natdynlink_run ignores undefined symbols: https://github.com/ocaml-flambda/flambda-backend/blob/main/ocaml/runtime/dynlink_nat.c#L108-L131 . This seems like bad behaviour - is it ever possible for those symbols to be undefined in a dynlinked cmxs? Should we just error if they are not found?

@gretay-js
Copy link
Copy Markdown
Contributor

gretay-js commented Nov 29, 2022

It looks like this wasn't caught earlier because caml_natdynlink_run ignores undefined symbols: https://github.com/ocaml-flambda/flambda-backend/blob/main/ocaml/runtime/dynlink_nat.c#L108-L131 . This seems like bad behaviour - is it ever possible for those symbols to be undefined in a dynlinked cmxs? Should we just error if they are not found?

We should at least print something about it with the debug runtime.

mshinwell added a commit to mshinwell/oxcaml that referenced this pull request Dec 13, 2022
@mshinwell
Squashed 'ocaml/' changes from 5bf2820..06c189a
76de3ce 
06c189a flambda-backend: Make stack allocation the default (oxcaml#1013)
98debd5 flambda-backend: Initial support for value slots not of value kind (oxcaml#946)
deb1714 flambda-backend: Add is_last flag to closinfo words (oxcaml#938)
d07fce1 flambda-backend: Disable poll insertion in Configure (oxcaml#967)
0f1ce0e flambda-backend: Regenerate ocaml/configure autoconf 2.69 (instead of 2.71) (oxcaml#1012)
27132d8 flambda-backend: Fix for spurious typing error related to expanding through functor arguments (oxcaml#997)
724fb68 flambda-backend: Use `Compilation_unit.t` instead of `Ident.t` for globals (oxcaml#871)
396d5b8 flambda-backend: Add a test for frametable setup in natdynlinked libraries (oxcaml#983)
b73ab12 flambda-backend: Fix invocation of `caml_shared_startup` in native dynlink (oxcaml#980)
7c7d75a flambda-backend: Fix split_default_wrapper which did not trigger anymore with flambda2 (oxcaml#970)
8fb75bd flambda-backend: Port ocaml/ocaml#11727 and ocaml/ocaml#11732 (oxcaml#965)
fdb7987 flambda-backend: Fix include functor issue after 4.14 merge. (oxcaml#948)
9745cdb flambda-backend: Print -dprofile/-dtimings output to stdout like 4.12 (oxcaml#943)
5f51f21 flambda-backend: Merge pull request oxcaml#932 from mshinwell/4.14-upgrade
841687d flambda-backend: Run make alldepend in ocaml/ (oxcaml#936)
72a7658 flambda-backend: Remove reformatting changes only in dynlink/dune (preserving PR889 and adjusting to minimise diff)
6d758cd flambda-backend: Revert whitespace changes in dune files, to match upstream
c86bf6e flambda-backend: Remove duplicate tests for polling
971dbeb flambda-backend: Testsuite fixes
32f8356 flambda-backend: Topeval fix for symbols patch
befea01 flambda-backend: Compilation fixes / rectify merge faults
a84543f flambda-backend: Merge ocaml-jst
8e65056 flambda-backend: Merge ocaml-jst
4d70045 flambda-backend: Remove filename from system frametable (amd64) (oxcaml#920)
5e57b7d flambda-backend: Bugfix for runtime frame_descr logic for C frames (oxcaml#918)
6423d5e flambda-backend: Merge pull request oxcaml#914 from mshinwell/merge-ocaml-jst-2022-10-24
ead605c flambda-backend: Add a missing Extract_exception (oxcaml#916)
c8f1481 flambda-backend: Resolve conflicts and add specialise/specialised attributes to Builtin_attributes
cf4d0d3 flambda-backend: Merge fixes (#21)
c2f742f flambda-backend: Re-enable some tests for Flambda2 (oxcaml#881)
3d38d13 flambda-backend: Long frames in frametable (oxcaml#797)
85aec7b flambda-backend: Add loop attribute to Builtin_attributes
c0f16e3 flambda-backend: Compilation fixes
90dea23 flambda-backend: Merge flambda-backend/main
5acc6ea flambda-backend: Fixes after merge
e501946 flambda-backend: Merge ocaml-jst
115083b flambda-backend: Merge ocaml-jst
9943b2e flambda-backend: Revert "Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"" (oxcaml#909)
ce339f1 flambda-backend: Fix alloc modes and call kinds for overapplications (oxcaml#902)
e6a317c flambda-backend: Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"
853c488 flambda-backend: Transform tail-recursive functions into recursive continuations (oxcaml#893)
5a977e4 flambda-backend: Fix missing End_region primitives on switch arms (oxcaml#898)
7fa7f9d flambda-backend: Add missing dependencies to Dune files (oxcaml#889)
3cd36f0 flambda-backend: Have Lambda `Pgetglobal` and `Psetglobal` take `Compilation_unit.t` (oxcaml#896)
7565915 flambda-backend: [@poll error] attribute (oxcaml#745)
9eb9448 flambda-backend: Backport the main safepoints PRs (oxcaml#740)
689bdda flambda-backend: Add strict mode for ocamldep (oxcaml#892)

git-subtree-dir: ocaml
git-subtree-split: 06c189a
mshinwell added a commit to mshinwell/oxcaml that referenced this pull request Dec 13, 2022
@mshinwell
Squashed 'ocaml/' changes from 5bf2820..06c189a
b273eee 
06c189a flambda-backend: Make stack allocation the default (oxcaml#1013)
98debd5 flambda-backend: Initial support for value slots not of value kind (oxcaml#946)
deb1714 flambda-backend: Add is_last flag to closinfo words (oxcaml#938)
d07fce1 flambda-backend: Disable poll insertion in Configure (oxcaml#967)
0f1ce0e flambda-backend: Regenerate ocaml/configure autoconf 2.69 (instead of 2.71) (oxcaml#1012)
27132d8 flambda-backend: Fix for spurious typing error related to expanding through functor arguments (oxcaml#997)
724fb68 flambda-backend: Use `Compilation_unit.t` instead of `Ident.t` for globals (oxcaml#871)
396d5b8 flambda-backend: Add a test for frametable setup in natdynlinked libraries (oxcaml#983)
b73ab12 flambda-backend: Fix invocation of `caml_shared_startup` in native dynlink (oxcaml#980)
7c7d75a flambda-backend: Fix split_default_wrapper which did not trigger anymore with flambda2 (oxcaml#970)
8fb75bd flambda-backend: Port ocaml/ocaml#11727 and ocaml/ocaml#11732 (oxcaml#965)
fdb7987 flambda-backend: Fix include functor issue after 4.14 merge. (oxcaml#948)
9745cdb flambda-backend: Print -dprofile/-dtimings output to stdout like 4.12 (oxcaml#943)
5f51f21 flambda-backend: Merge pull request oxcaml#932 from mshinwell/4.14-upgrade
841687d flambda-backend: Run make alldepend in ocaml/ (oxcaml#936)
72a7658 flambda-backend: Remove reformatting changes only in dynlink/dune (preserving PR889 and adjusting to minimise diff)
6d758cd flambda-backend: Revert whitespace changes in dune files, to match upstream
c86bf6e flambda-backend: Remove duplicate tests for polling
971dbeb flambda-backend: Testsuite fixes
32f8356 flambda-backend: Topeval fix for symbols patch
befea01 flambda-backend: Compilation fixes / rectify merge faults
a84543f flambda-backend: Merge ocaml-jst
8e65056 flambda-backend: Merge ocaml-jst
4d70045 flambda-backend: Remove filename from system frametable (amd64) (oxcaml#920)
5e57b7d flambda-backend: Bugfix for runtime frame_descr logic for C frames (oxcaml#918)
6423d5e flambda-backend: Merge pull request oxcaml#914 from mshinwell/merge-ocaml-jst-2022-10-24
ead605c flambda-backend: Add a missing Extract_exception (oxcaml#916)
c8f1481 flambda-backend: Resolve conflicts and add specialise/specialised attributes to Builtin_attributes
cf4d0d3 flambda-backend: Merge fixes (#21)
c2f742f flambda-backend: Re-enable some tests for Flambda2 (oxcaml#881)
3d38d13 flambda-backend: Long frames in frametable (oxcaml#797)
85aec7b flambda-backend: Add loop attribute to Builtin_attributes
c0f16e3 flambda-backend: Compilation fixes
90dea23 flambda-backend: Merge flambda-backend/main
5acc6ea flambda-backend: Fixes after merge
e501946 flambda-backend: Merge ocaml-jst
115083b flambda-backend: Merge ocaml-jst
9943b2e flambda-backend: Revert "Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"" (oxcaml#909)
ce339f1 flambda-backend: Fix alloc modes and call kinds for overapplications (oxcaml#902)
e6a317c flambda-backend: Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"
853c488 flambda-backend: Transform tail-recursive functions into recursive continuations (oxcaml#893)
5a977e4 flambda-backend: Fix missing End_region primitives on switch arms (oxcaml#898)
7fa7f9d flambda-backend: Add missing dependencies to Dune files (oxcaml#889)
3cd36f0 flambda-backend: Have Lambda `Pgetglobal` and `Psetglobal` take `Compilation_unit.t` (oxcaml#896)
7565915 flambda-backend: [@poll error] attribute (oxcaml#745)
9eb9448 flambda-backend: Backport the main safepoints PRs (oxcaml#740)
689bdda flambda-backend: Add strict mode for ocamldep (oxcaml#892)

git-subtree-dir: ocaml
git-subtree-split: 06c189a
mshinwell added a commit to mshinwell/oxcaml that referenced this pull request Dec 28, 2022
@mshinwell
Squashed 'ocaml/' changes from 5bf2820..b11eea1
f97a773 
b11eea1 flambda-backend: Introduce Import_info (oxcaml#1036)
bc5b135 flambda-backend: Fix `ocamlobjinfo` on flambda2 .cmx files (oxcaml#1029)
c8babbd flambda-backend: Compilation_unit optimisations (oxcaml#1035)
e8d3e22 flambda-backend: Use 4.14.0 opam switch for building (includes upgrading ocamlformat to 0.24.1) (oxcaml#1030)
eb14a86 flambda-backend: Port PR81 from ocaml-jst (oxcaml#1024)
131bc12 flambda-backend: Merge ocaml-jst 2022-12-13 (oxcaml#1022)
06c189a flambda-backend: Make stack allocation the default (oxcaml#1013)
98debd5 flambda-backend: Initial support for value slots not of value kind (oxcaml#946)
deb1714 flambda-backend: Add is_last flag to closinfo words (oxcaml#938)
d07fce1 flambda-backend: Disable poll insertion in Configure (oxcaml#967)
0f1ce0e flambda-backend: Regenerate ocaml/configure autoconf 2.69 (instead of 2.71) (oxcaml#1012)
27132d8 flambda-backend: Fix for spurious typing error related to expanding through functor arguments (oxcaml#997)
724fb68 flambda-backend: Use `Compilation_unit.t` instead of `Ident.t` for globals (oxcaml#871)
396d5b8 flambda-backend: Add a test for frametable setup in natdynlinked libraries (oxcaml#983)
b73ab12 flambda-backend: Fix invocation of `caml_shared_startup` in native dynlink (oxcaml#980)
7c7d75a flambda-backend: Fix split_default_wrapper which did not trigger anymore with flambda2 (oxcaml#970)
8fb75bd flambda-backend: Port ocaml/ocaml#11727 and ocaml/ocaml#11732 (oxcaml#965)
fdb7987 flambda-backend: Fix include functor issue after 4.14 merge. (oxcaml#948)
9745cdb flambda-backend: Print -dprofile/-dtimings output to stdout like 4.12 (oxcaml#943)
5f51f21 flambda-backend: Merge pull request oxcaml#932 from mshinwell/4.14-upgrade
841687d flambda-backend: Run make alldepend in ocaml/ (oxcaml#936)
72a7658 flambda-backend: Remove reformatting changes only in dynlink/dune (preserving PR889 and adjusting to minimise diff)
6d758cd flambda-backend: Revert whitespace changes in dune files, to match upstream
c86bf6e flambda-backend: Remove duplicate tests for polling
971dbeb flambda-backend: Testsuite fixes
32f8356 flambda-backend: Topeval fix for symbols patch
befea01 flambda-backend: Compilation fixes / rectify merge faults
a84543f flambda-backend: Merge ocaml-jst
8e65056 flambda-backend: Merge ocaml-jst
4d70045 flambda-backend: Remove filename from system frametable (amd64) (oxcaml#920)
5e57b7d flambda-backend: Bugfix for runtime frame_descr logic for C frames (oxcaml#918)
6423d5e flambda-backend: Merge pull request oxcaml#914 from mshinwell/merge-ocaml-jst-2022-10-24
ead605c flambda-backend: Add a missing Extract_exception (oxcaml#916)
c8f1481 flambda-backend: Resolve conflicts and add specialise/specialised attributes to Builtin_attributes
cf4d0d3 flambda-backend: Merge fixes (#21)
c2f742f flambda-backend: Re-enable some tests for Flambda2 (oxcaml#881)
3d38d13 flambda-backend: Long frames in frametable (oxcaml#797)
85aec7b flambda-backend: Add loop attribute to Builtin_attributes
c0f16e3 flambda-backend: Compilation fixes
90dea23 flambda-backend: Merge flambda-backend/main
5acc6ea flambda-backend: Fixes after merge
e501946 flambda-backend: Merge ocaml-jst
115083b flambda-backend: Merge ocaml-jst
9943b2e flambda-backend: Revert "Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"" (oxcaml#909)
ce339f1 flambda-backend: Fix alloc modes and call kinds for overapplications (oxcaml#902)
e6a317c flambda-backend: Revert "Transform tail-recursive functions into recursive continuations (oxcaml#893)"
853c488 flambda-backend: Transform tail-recursive functions into recursive continuations (oxcaml#893)
5a977e4 flambda-backend: Fix missing End_region primitives on switch arms (oxcaml#898)
7fa7f9d flambda-backend: Add missing dependencies to Dune files (oxcaml#889)
3cd36f0 flambda-backend: Have Lambda `Pgetglobal` and `Psetglobal` take `Compilation_unit.t` (oxcaml#896)
7565915 flambda-backend: [@poll error] attribute (oxcaml#745)
9eb9448 flambda-backend: Backport the main safepoints PRs (oxcaml#740)
689bdda flambda-backend: Add strict mode for ocamldep (oxcaml#892)

git-subtree-dir: ocaml
git-subtree-split: b11eea1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshinwell mshinwell mshinwell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lukemaurer @stedolan @gretay-js @mshinwell