This looks correct, and should indeed fix the afl instrumentation. However, I wonder why the fact that afl instrumentation was broken with flambda2 was not detected by running the testsuite (which contains a test for afl instrumentation) ?

See #1197

The .ml test file used for AFL only works with the reference output under flambda2 if the parts using lazy are commented out. This is probably related to the different compilation strategy used for lazy values under flambda2 in Translcore.

Should we merge this with the lazy part commented out, and see later if we can fix it ? Or fix it before merging this PR ?

I spoke to @stedolan about this and it seems like we need to investigate further what's causing the discrepancy with lazy. It doesn't seem to be (directly) related to the different compilation path for lazy prior to the middle-end, as there is a different path used for AFL in all cases.

I'm going to merge the additions to the Flambda 2 code so this PR can be closed, and the support is there. As far as I can see the AFL test cases pass on my macOS machine - it seems like there may be some problem with the afl++ package used for the Linux CI. We can investigate that separately. I've had one other report of these tests being problematic on Linux too.

The lazy test is still failing on Linux (and I spent a few hours trying to debug it and fix it, not realizing that this conversation already happened). I'm fairly sure it's related to the GC. That is, running the GC shows up in the AFL instrumentation. I put more detail in this PR: #1789

Stephen doesn't want to merge #1789 because running the GC shouldn't affect instrumentation. But it's costly (in terms of developer time) and confusing to have a failing test run as part of the build. Does anyone have ideas as to why GC would show up in the AFL instrumentation?

TL;DR this is the same issue as stedolan/crowbar#14. The fix in ocaml/ocaml#1754 doesn't seem to be applying for some reason I don't understand...

For the input program:

let laziness () =
  let _ = Lazy.force already_forced in
  Gc.major ()

Pseudocode for the instrumented CMM is morally something like this in flambda2:

trigger_afl_event "A";
let tag = caml_obj_tag already_forced in
let _ =
  if tag = 501 then ( (* Forward_tag = 250 *)
    trigger_afl_event "B"
  ) else (
    trigger_afl_event "C";
    if tag <> 493 then ( (* Lazy_tag = 246 *)
      trigger_afl_event "D";
    ) else (
      trigger_afl_event "E";
      CamlinternalLazy.force_lazy_block already_forced
    )
in
caml_gc_major ()

On the first run, this triggers: A, B.
On the second run, this triggers: A, C, D.

In other words, the reason the test fails is:

• Running the GC changes the tag from Forward_tag to the tag of the thing inside the lazy (here, 0).
• Flambda 2 inlines a different code path for Forward_tag vs. non-Forward_tag. (In contrast, closure always calls CamlinternalLazy.force when afl-instrument is set, so the same branches of instrumented code are explored.)

The fix in ocaml/ocaml#1754 has ap_inlined=Default_inlined; on the call to CamlinteralLazy.force that's inserted when -afl-instrument is in force — I think that enables flambda2 to inline the definition (and thus -afl-instrument to instrument the cmm). Changing this to Never_inlined allows the test to pass. I'll open a PR for that.