Extend record punning to allow destructuring. by yallop · Pull Request #3 · ocaml/ocaml

yallop · 2014-01-30T11:51:58Z

This patch extends the record punning syntax (b01621e) to allow simultaneous label punning and destructuring. Variables bound using as at the top level of a field pattern are treated as labels. For example, it allows you to write

   fun { { x ; y } as p; q } -> e

which is equivalent to

   fun { p = { x ; y } as p; q } -> e

mmottl · 2014-01-30T13:55:07Z

I like the proposal. This syntax change is non-intrusive, intuitive, and would help writing complex pattern matching rules. I sure have some code that could benefit from it.

samoht · 2014-01-30T14:14:27Z

Would be nice if this syntax was also able to fix cases such:

type t = { x: int; y: int }
let f { x as t } = t.y

So would be nice if we can rewrite your example to something which looks like:

fun { { x ; y as p } ; q } -> e

hcarty · 2014-01-30T14:15:31Z

I have wanted this or something similar in the past when using nested records. If you change as p to as not_a_field_name would you still get Error: Unbound record field not_a_field_name with proper location information?

pw374 · 2014-01-30T14:19:15Z

On 30 Jan 2014, at 14:14 pm, Thomas Gazagnaire [email protected] wrote:

Would be nice if this syntax was also able to fix cases such:

type t = { x: int; y: int }
let f { x as t } = t.y
So would be nice if we can rewrite your example to something which looks like:

fun { { x ; y as p } ; q } -> e

I can see that it saves a few characters but it’s not worth it in my opinion.
This syntax feels very odd, because in { x as t } it’s not x that’s as t, but the entire record.
I don’t like it, to me it’s somewhat misleading and unnatural.

yallop · 2014-01-30T14:23:02Z

@hcarty: yes, location information and error messages are preserved.

# fun { { x ; y } as not_a_field_name; q } -> ();;
Characters 19-35:
  fun { { x ; y } as not_a_field_name; q } -> ();;
                     ^^^^^^^^^^^^^^^^
Error: Unbound record field not_a_field_name

mmottl · 2014-01-30T14:23:50Z

@samoht: not bad, but you'd only save having to type an extra pair of parentheses then. The "as" syntax as such already saves you having to duplicate a (potentially long) field name. The "x as t" looks a little strange to me. I think I'd prefer having the binding outside of the enclosing structure as in the first proposal.

samoht · 2014-01-30T14:26:38Z

Regarding my proposition, it's exactly the same as for tuples (which already works):

let f (x, _ as t) = snd t

mmottl · 2014-01-30T14:29:47Z

@samoht: true, but that's because the tuple syntax, unlike records, allows you to drop parentheses in the first place. The enclosing parentheses here aren't required by the tuple, only by the "as"-binding.

Update: to be precise, the tuple does require the parentheses here, because it's a function definition, but not e.g. in:

let x, _ as t = ...

pw374 · 2014-01-30T14:33:35Z

@samoht wrote:

Regarding my proposition, it's exactly the same as for tuples (which already works):
let f (x, _ as t) = snd t

But that’s between parenthesis, not curly braces.
The “as” takes "everything" on its left side, no matter if it’s a tuple or an or-pattern
(as in

# let f = function (`A | `B (_, _) as t) -> t;;
val f : [< `A | `B of 'a * 'b ] -> [> `A | `B of 'a * 'b ] = <fun>

the "as t" takes everything of its left)

samoht · 2014-01-30T14:45:02Z

But that’s between parenthesis, not curly braces.

I'm just proposing to have a consistent notation for tuple and records. But I see your point, so I stop arguing :-)

pw374 · 2014-01-30T14:49:20Z

I believe I’ve found a better and stronger reason:
in
function { contents = (A | B) as x } -> x
is it the whole record that is as x, or just the value of the field contents? ;-)

alainfrisch · 2014-01-30T15:27:24Z

Another useful extension of the syntax for record patterns would be:

{ x.a = pa; x.b = pb; x.c; y.t = pt }

equivalent to:

{ x = {a = pa; b = pb; c}; y = {t = pt} };

This would also be useful for record override:

{ r with x.a = ea; x.b = eb; x.c; y.t = et }

equivalent to:

{r with x = {r.x with a = ea; b = eb; c}; y = {r.y with t = et}}

Kakadu · 2014-01-31T12:31:36Z

I'm on @pw374 's side. I don't like

type t = { x: int; y: int }
let f { x as t } = t.y

It will be great if 2nd line will be parsed as equivalent to let f {y; x as t } = y (t should be a filed of record, not the whole record). Probably idea this change is inspired by camlp4r but I still don't like. If we apply it line let f { y; x as t } = t.y will look strange for me.

gasche · 2014-01-31T16:50:29Z

If I can indulge in a small meta note, I think community review works best for proposal that have a good chance of being consensual. As soon as something touches the language definition, the design space explodes and it's very hard to reach agreement (whether in small or larger groups) -- it's one of the reason why maintainers are generally so reluctant with syntax changes. While I don't think Jeremy's initial suggestion was unreasonable, I'll personally prudently avoid trying to obtain an actual decision on this, at least for now.

pw374 · 2014-01-31T16:56:46Z

On 31 Jan 2014, at 16:50, @gasche wrote:

If I can indulge in a small meta note, I think community review works best for proposal that have a good chance of being consensual. As soon as something touches the language definition, the design space explodes and it's very hard to reach agreement (whether in small or larger groups) -- it's one of the reason why maintainers are generally so reluctant with syntax changes. While I don't think Jeremy's initial suggestion was unreasonable, I'll personally prudently avoid trying to obtain an actual decision on this, at least for now.

I do like @yallop's initial suggestion. I believe no one on this thread has shown to be against it.

It’s a tiny patch (only 2 reasonable new lines in parsing/parser.mly) and it doesn’t seem to introduce any ambiguity on the syntax.

mmottl · 2014-01-31T17:12:27Z

Let me add some support for the first proposal, too. I think it at least deserves a short discussion.

garrigue · 2014-02-01T10:03:59Z

I kind of wonder whether this change (the initial proposal) is worthwhile.
Namely, in { { x ; y } as p; q } it is not very intuitive that p is a field name.
Wouldn't it end up obfuscating the code, for a very small change in the number of characters?
This is just a consequence of the fact the alias comes after the aliased pattern in ML, which was probably not a good idea to start with, but we're stuck with that.

gasche · 2014-02-01T10:10:51Z

@garrigue : couldn't we fix this by interpreting as as symmetrical pattern intersection, allowing <pattern> as <simple pattern>, or at least an additional <variable> as <simple pattern>?

damiendoligez · 2015-02-09T18:04:33Z

@gasche I've always wanted to write x as <pattern> instead of <pattern> as x, so this addition would feel very natural to me. Now adding it to the grammar, that's another can of worms...

git-svn-id: http://caml.inria.fr/svn/ocaml/version/4.02@15985 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02

(Merge of branch cmm-mach-types and PR#115.) - Register type "Addr" is split into . "Val" (well-formed OCaml values, appropriate as GC roots) . "Addr" (derived pointers within the heap, must not survive a GC) - memory_chunk "Word" is split into . "Word_val" (OCaml value) . "Word_int" (native-sized integer, not a pointer into the heap) Cmmgen was updated to use Word_val or Word_int as appropriate. Application #1: fail at compile-time if a derived pointer within the heap survives a GC point (cf. PR#6484). Application #2: CSE can do a better job across allocation points (keep factoring expressions of type Int, Val, Float, but not Addr). Application #3: slightly fewer roots given to the GC (e.g. pointers into bigarray data). git-svn-id: http://caml.inria.fr/svn/ocaml/trunk@16269 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02

Fix assertion failure for 'effect E _ ->'

Add Gc.print_stat hook to std_exit.ml

gasche · 2015-11-19T15:42:04Z

We discussed this PR at the developer meeting yesterday, and there was opposition (not from me) to including it without a patch allowing to write at least x as <pattern> (or possibly p1 as p2 for general intersection) to use a more natural order.

(We have no guarantees, of course, that a proposal including this richer syntax would be accepted. I would personally support¹ p1 as p2 independently of the present change.)

¹: another thing I would like to have is for the individual patterns in p1 | p2 have variables that do not appear in the other pattern (but only allow to use variables from the intersection in the right-hand-side). Possibly we could restrict this to variables prefixed with _. The idea is that naming things can clarify, even when they are unused.

yallop · 2015-11-23T11:17:23Z

Thanks for the feedback. I'll investigate supporting the more general syntax.

another think I would like to have is for the individual patterns in p1 | p2 have variables that do not appear in the other pattern (but only allow to use variables from the intersection in the right-hand-side).

I'd like to have that as well. The old patterns Camlp4 extension used to support that behaviour, IIRC. Even more ambitiously, it'd be good to have something similar for GADTs, if they're ever updated to support |-patterns -- i.e. to have p1 | p2 to introduce the constraints that are in the intersection. (However, both these extensions are beyond the scope of this PR, which has already grown rather significantly.)

Fixes ocaml#3.

fun { { x ; y } as p; q } -> e is equivalent to fun { p = { x ; y } as p; q } -> e

Avoid deleting instrument.mlp

Use the new file location for emit.mlp files

* Code compiles and links successfully

add internationalisation support

Embed HTML files to serve the documentation

Reported by `-fsanitize=memory`: ``` > ==102752==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x7f2ba7fb4ea4 in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:496:18 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was stored to memory at > #0 0x7f2ba7fb4e9d in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:497:69 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was created by an allocation of 'buf' in the stack frame > #0 0x7f2ba7fb3dbc in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:402:7 > ``` This is in fact an EV_LIFECYCLE with EV_RING_STOP, which has 0 additional data, and thus msg_length 2: ``` runtime/runtime_events.c: EV_RUNTIME, (ev_message_type){.runtime=EV_LIFECYCLE}, EV_RING_STOP, 0, ``` Attempting to read from `buf[2]` would read uninitialized data (or potentially beyond the end of the buffer). Check `msg_length` before reading. Signed-off-by: Edwin Török <[email protected]>

Bigarrays are printed in the toplevel as `<abstr>`, but ObjTbl.mem computes a hash. However bigarrays are uninitialized when allocated, so this seems to be a genuine bug: ``` ==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 ocaml#2 0x599ebf in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1065:14 ocaml#3 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#4 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#5 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#6 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 ocaml#2 0x4e801f in caml_ba_create /var/home/edwin/git/ocaml/runtime/bigarray.c:673:10 ocaml#3 0x59b8fc in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 ocaml#4 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#5 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#6 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#7 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 ``` Suppress it for now until a solution is found. The testsuite passes now: ``` Summary: 1334 tests passed 103 tests skipped 0 tests failed 0 tests not started (parent test skipped or failed) 0 unexpected errors 1437 tests considered ``` Signed-off-by: Edwin Török <[email protected]>

Reported by `-fsanitize=memory`: ``` > ==102752==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x7f2ba7fb4ea4 in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:496:18 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was stored to memory at > #0 0x7f2ba7fb4e9d in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:497:69 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was created by an allocation of 'buf' in the stack frame > #0 0x7f2ba7fb3dbc in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:402:7 > ``` This is in fact an EV_LIFECYCLE with EV_RING_STOP, which has 0 additional data, and thus msg_length 2: ``` runtime/runtime_events.c: EV_RUNTIME, (ev_message_type){.runtime=EV_LIFECYCLE}, EV_RING_STOP, 0, ``` Attempting to read from `buf[2]` would read uninitialized data (or potentially beyond the end of the buffer). Check `msg_length` before reading. Signed-off-by: Edwin Török <[email protected]>

Bigarrays are printed in the toplevel as `<abstr>`, but ObjTbl.mem computes a hash. However bigarrays are uninitialized when allocated, so this seems to be a genuine bug: ``` ==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 ocaml#2 0x599ebf in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1065:14 ocaml#3 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#4 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#5 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#6 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 ocaml#2 0x4e801f in caml_ba_create /var/home/edwin/git/ocaml/runtime/bigarray.c:673:10 ocaml#3 0x59b8fc in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 ocaml#4 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#5 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#6 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#7 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 hashing is only needed to avoid recursion, skip it when the OCaml value doesn't contain further nested OCaml values, i.e. when it wouldn't be scanned by the GC. The testsuite passes now: ``` Summary: 1335 tests passed 102 tests skipped 0 tests failed 0 tests not started (parent test skipped or failed) 0 unexpected errors 1437 tests considered ``` Suggested-by: Gabriel Scherer <[email protected]> Signed-off-by: Edwin Török <[email protected]>

Found by -fsanitize=memory -fsanitize-memory-track-origins: ``` > ==102752==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x7f2ba7fb4ea4 in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:496:18 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was stored to memory at > #0 0x7f2ba7fb4e9d in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:497:69 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was created by an allocation of 'buf' in the stack frame > #0 0x7f2ba7fb3dbc in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:402:7 > ``` This is in fact an EV_LIFECYCLE with EV_RING_STOP, which has 0 additional data, and thus msg_length 2: ``` runtime/runtime_events.c: EV_RUNTIME, (ev_message_type){.runtime=EV_LIFECYCLE}, EV_RING_STOP, 0, ``` Attempting to read from `buf[2]` would read uninitialized data. Signed-off-by: Edwin Török <[email protected]>

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 ocaml#2 0x599ebf in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1065:14 ocaml#3 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#4 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#5 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#6 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 ocaml#2 0x4e801f in caml_ba_create /var/home/edwin/git/ocaml/runtime/bigarray.c:673:10 ocaml#3 0x59b8fc in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 ocaml#4 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 ocaml#5 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 ocaml#6 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#7 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) ocaml#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 <[email protected]> Signed-off-by: Edwin Török <[email protected]>

Found by -fsanitize=memory -fsanitize-memory-track-origins: ``` > ==102752==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x7f2ba7fb4ea4 in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:496:18 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was stored to memory at > #0 0x7f2ba7fb4e9d in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:497:69 > #1 0x7f2ba7fbc016 in caml_ml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:1207:9 > ocaml#2 0x59ba5c in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 > ocaml#3 0x5a9220 in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 > ocaml#4 0x540d6b in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 > ocaml#5 0x7f2ba8120087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#6 0x7f2ba812014a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) > ocaml#7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 617637580ee48eff08a2bce790e1667ad09f3b69) > > Uninitialized value was created by an allocation of 'buf' in the stack frame > #0 0x7f2ba7fb3dbc in caml_runtime_events_read_poll /var/home/edwin/git/ocaml/otherlibs/runtime_events/runtime_events_consumer.c:402:7 > ``` This is in fact an EV_LIFECYCLE with EV_RING_STOP, which has 0 additional data, and thus msg_length 2: ``` runtime/runtime_events.c: EV_RUNTIME, (ev_message_type){.runtime=EV_LIFECYCLE}, EV_RING_STOP, 0, ``` Attempting to read from `buf[2]` would read uninitialized data. Signed-off-by: Edwin Török <[email protected]>

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 <[email protected]> Signed-off-by: Edwin Török <[email protected]> Co-authored-by: Edwin Török <[email protected]>

yallop pushed a commit to yallop/ocaml that referenced this pull request Mar 18, 2015

Fix ocaml#3: bind implicit args while processing type declarations

1d7f380

stedolan pushed a commit to stedolan/ocaml that referenced this pull request Aug 18, 2015

Merge pull request ocaml#3 from yallop/effect-underscore

07f67d1

Fix assertion failure for 'effect E _ ->'

mshinwell referenced this pull request in mshinwell/ocaml Oct 22, 2015

Merge pull request #3 from AltGr/flambda_trunk

9c32bff

Add Gc.print_stat hook to std_exit.ml

damiendoligez added this to the 4.04-or-later milestone Jan 12, 2016

Drup added a commit to Drup/ocaml that referenced this pull request Apr 27, 2016

Use Ident tables inside components

2f60492

Fixes ocaml#3.

Allow 'as' variables to bind labels in pattens. For example

42f6bc0

fun { { x ; y } as p; q } -> e is equivalent to fun { p = { x ; y } as p; q } -> e

yallop force-pushed the as-label-binding branch from 7107f8b to 42f6bc0 Compare June 17, 2016 19:59

This was referenced Mar 14, 2019

Re: [Caml-list] Ocamlc stack overflow #8384

Closed

AMD Opteron: crash in caml_oldify_mopup #8449

Closed

ocaml broken on FreeBSD sparc64 #8467

Closed

Octachron referenced this pull request in Octachron/ocaml Jul 29, 2019

Merge pull request #3 from Octachron/update_makefile

9134538

Avoid deleting instrument.mlp

jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020

Fix tests ocaml#3

1fcd6d4

jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020

Fix tests ocaml#3

a637d8b

jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020

Fix tests ocaml#3

a3bf2b0

jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 5, 2020

Fix tests ocaml#3

1c4e013

gretay-js pushed a commit to gretay-js/ocaml that referenced this pull request May 28, 2021

Merge pull request ocaml#3 from gretay-js/fix_directory_name_in_emit

b0c5105

Use the new file location for emit.mlp files

poechsel pushed a commit to poechsel/ocaml that referenced this pull request Jul 2, 2021

Use the new file location for emit.mlp files (ocaml#3)

24251cf

andreas-schwab mentioned this pull request Nov 28, 2022

ocamlopt.opt crashes from stack overflow while building coccinelle on RISC-V #11765

Closed

shindere mentioned this pull request Mar 22, 2023

Use a dedicated type to represent global identifiers in CMO files #12031

Merged

DMaroo added a commit to DMaroo/ocaml that referenced this pull request Dec 16, 2023

working commit ocaml#3

a9c4ce4

* Code compiles and links successfully

EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024

Merge pull request ocaml#3 from patricoferris/i18n

fdbaf5d

add internationalisation support

EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024

Merge pull request ocaml#3 from tmattio/embed-html

f54781e

Embed HTML files to serve the documentation

NickBarnes mentioned this pull request Feb 29, 2024

Statmemprof resurrected #12923

Merged

shindere mentioned this pull request Apr 15, 2024

Emancipate dynlink from compilerlibs #11996

Merged

Octachron added a commit that referenced this pull request Sep 19, 2024

Unify mark and direction (#3)

9eadfb8

za3k mentioned this pull request Mar 6, 2025

Improve documentation for Printexc backtraces #13822

Open

Conversation

yallop commented Jan 30, 2014

Uh oh!

mmottl commented Jan 30, 2014

Uh oh!

samoht commented Jan 30, 2014

Uh oh!

hcarty commented Jan 30, 2014

Uh oh!

pw374 commented Jan 30, 2014

Uh oh!

yallop commented Jan 30, 2014

Uh oh!

mmottl commented Jan 30, 2014

Uh oh!

samoht commented Jan 30, 2014

Uh oh!

mmottl commented Jan 30, 2014

Uh oh!

pw374 commented Jan 30, 2014

Uh oh!

samoht commented Jan 30, 2014

Uh oh!

pw374 commented Jan 30, 2014

Uh oh!

alainfrisch commented Jan 30, 2014

Uh oh!

Kakadu commented Jan 31, 2014

Uh oh!

gasche commented Jan 31, 2014

Uh oh!

pw374 commented Jan 31, 2014

Uh oh!

mmottl commented Jan 31, 2014

Uh oh!

garrigue commented Feb 1, 2014

Uh oh!

gasche commented Feb 1, 2014

Uh oh!

damiendoligez commented Feb 9, 2015

Uh oh!

gasche commented Nov 19, 2015

Uh oh!

yallop commented Nov 23, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants