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

Invalid reads from complex bigarrays (simple version) #1755

Merged
merged 3 commits into from May 3, 2018

Conversation

Projects
None yet
2 participants
@xclerc
Copy link
Contributor

xclerc commented May 2, 2018

This PR contains the first/simple implementation of the fix mentioned in #1729.

@xavierleroy
Copy link
Contributor

xavierleroy left a comment

Looks good to me. Thanks!

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented May 3, 2018

If we cherry-pick to 4.07 -- and I think we should -- the Changes entry should be moved to the 4.07 section.

@xclerc

This comment has been minimized.

Copy link
Contributor Author

xclerc commented May 3, 2018

(Entry moved to 4.07.)

@xavierleroy xavierleroy merged commit cb20d9b into ocaml:trunk May 3, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

xavierleroy added a commit that referenced this pull request May 3, 2018

Invalid reads from complex bigarrays (#1755)
The following function can raise an Assert_failure in trunk:

let bug_example () =
  let open Bigarray in
  let work = Array1.create complex64 c_layout 1 in
  work.{0} <- { Complex. re=0.; im=0. };
  let result = work.{0}.Complex.re in (* alloc *)
  assert (result = 0.)

because the Complex.t value (corresponding to work.{0} on
line "alloc" ) is allocated after the address inside the bigarray
has been computed but before the reads actually occur.
The allocation can trigger a collection that then deallocates the
bigarray.

This commit moves the allocation of the Complex.t value so that
it occurs after the reads from the bigarray. Our understanding
is that only complex reads can allocate: non-complex reads do
not allocate, and writes never allocate.

It also changes the Cmm type for bigarray addressing from Int to Addr, marking the fact
that the returned pointer is a derived pointer that may not survive a GC.
@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented May 3, 2018

Merged (cb20d9b) and cherry-picked (7d4092b)

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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.