Skip to content

Immutable read#4

Closed
kayceesrk wants to merge 7 commits intotrunkfrom
immutable-read
Closed

Immutable read#4
kayceesrk wants to merge 7 commits intotrunkfrom
immutable-read

Conversation

@kayceesrk
Copy link
Copy Markdown
Owner

DO NOT MERGE

@lpw25
Copy link
Copy Markdown

lpw25 commented Oct 30, 2017

I see that you're using the immutable read annotations for more than the read barrier. Since I don't know precisely what property you require. I should point out that the notion of "immutable" in this patch means, corresponds to a read being immutable from the language's point of view. This guarantees:

  1. If you read from it twice it will return the same value
  2. The result of a read will not be affected by mutable write operations

However, it does not mean that there won't be "initialising writes" that affect the value being read (e.g. a recursively defined immutable record). It's important to be clear on this as some optimisations require the stronger property that no writes at all can affect the value read. In fact my current plan is to use a type like:

type read =
  | Pure
  | Immutable
  | Mutable

where Pure is for values (e.g. modules) which are never affected by initialising or mutating writes, whilst Immutable is for those which are not affected by mutating writes, but could be subject to initialising writes. In particular, I think on trunk there is a mutability flag on Iload and I believe that corresponds to Pure rather than Immutable for the purposes of instruction scheduling.

@stedolan
Copy link
Copy Markdown

For these purposes, I think the notion of "immutable" we need is "all writes happen-before all reads", with happens-before defined by the memory model (and including program order).

By the way, I'm not convinced that Pure values exist in OCaml. Recursive modules mean that initialising writes can happen even to module fields.

@lpw25
Copy link
Copy Markdown

lpw25 commented Oct 31, 2017

Recursive modules mean that initialising writes can happen even to module fields.

Actually, I don't think they can. IIUC the compilation scheme for recursive modules actually allocates both the module block and the blocks for the fields initially and then mutates the blocks for the fields rather than mutating the module block.

The aim is to make this branch the generic one that includes the type
information. Actual compilation choices will be made in their own
branches.
@kayceesrk kayceesrk closed this May 11, 2022
@kayceesrk
Copy link
Copy Markdown
Owner Author

Closing old PRs.

kayceesrk pushed a commit that referenced this pull request Aug 21, 2024
…l#13294)

The toplevel printer detects cycles by keeping a hashtable of values
that it has already traversed.

However, some OCaml runtime types (at least bigarrays) may be
partially uninitialized, and hashing them at arbitrary program points
may read uninitialized memory. In particular, the OCaml testsuite
fails when running with a memory-sanitizer enabled, as bigarray
printing results in reads to uninitialized memory:

```
==133712==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4e6d11 in caml_ba_hash /var/home/edwin/git/ocaml/runtime/bigarray.c:486:45
    #1 0x52474a in caml_hash /var/home/edwin/git/ocaml/runtime/hash.c:251:35
    #2 0x599ebf in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1065:14
    #3 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9
    #4 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3
    #5 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
    #6 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
    #7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37)

  Uninitialized value was created by a heap allocation
    #0 0x47d306 in malloc (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x47d306) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37)
    #1 0x4e7960 in caml_ba_alloc /var/home/edwin/git/ocaml/runtime/bigarray.c:246:12
    #2 0x4e801f in caml_ba_create /var/home/edwin/git/ocaml/runtime/bigarray.c:673:10
    #3 0x59b8fc in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14
    #4 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9
    #5 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3
    #6 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
    #7 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
    #8 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /var/home/edwin/git/ocaml/runtime/bigarray.c:486:45 in caml_ba_hash
```

The only use of hashing in genprintval is to avoid cycles, that is, it
is only useful for OCaml values that contain other OCaml values
(including possibly themselves). Bigarrays cannot introduce cycles,
and they are always printed as "<abstr>" anyway.

The present commit proposes to be more conservative in which values
are hashed by the cycle detector to avoid this issue: we skip hashing
any value with tag above No_scan_tag -- which may not contain any
OCaml values.

Suggested-by: Gabriel Scherer <gabriel.scherer@gmail.com>

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Co-authored-by: Edwin Török <edwin.torok@cloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants