hackwaly / ocamlearlybird

OCaml debug adapter
MIT License
208 stars 23 forks source link

Add opaque value inspection #53

Closed hackwaly closed 8 months ago

hackwaly commented 10 months ago

Close #52

TODO:

image
hackwaly commented 10 months ago

Realuse feedback: sometimes segfault occurs

hackwaly commented 8 months ago

Realuse feedback: sometimes segfault occurs

fixed.

sim642 commented 8 months ago

When exactly do these appear? I haven't used earlybird for a while.

I just tried it on a tiny dummy program and found Global → Stdlib → stdout to be one of these opaque blocks with two opaque blocks inside. But when I try to look into them, the debugger still seems to segfault and End_of_file somehow appears as the value.

hackwaly commented 8 months ago

When exactly do these appear? I haven't used earlybird for a while.

I just tried it on a tiny dummy program and found Global → Stdlib → stdout to be one of these opaque blocks with two opaque blocks inside. But when I try to look into them, the debugger still seems to segfault and End_of_file somehow appears as the value.

Previously Scene.get_tag miss a if not (is_block rv) then Lwt.return Obj.int_tag guard. That cause segfault while inspecting opaque block with int field. I push the commit with --force, did you confirm your test version with that change?

sim642 commented 8 months ago

I noticed the force push, the get_tag change is there.

I looked into it a bit and think I've figured it out: stdout is some particularly special stuff coming from the OCaml runtime, it uses custom_tag. Blocks with custom_tag and abstract_tag have completely arbitrary contents (not structured into a number of block fields) and shouldn't be looked into at all because they can contain arbitrary out-of-heap pointers and cannot contain any OCaml heap references. I pushed a fix to skip those.

hackwaly commented 8 months ago

Shall we release this with those TODO(s)? I think it's a big improvement.

sim642 commented 8 months ago

I think the TODOs are fine, all the most common things are already covered.

I'll try this on a larger project in case some other issue pops up. If it seems fine, I'll merge this and can do a release.

hackwaly commented 8 months ago

There's extra object_tag, infix_tag, forward_tag and no_scan_tag not handled. I don't know the meanings of those tags. Maybe we should handle those tags as well.

let lazy_tag = 246
let closure_tag = 247
let object_tag = 248
let infix_tag = 249
let forward_tag = 250

let no_scan_tag = 251

let abstract_tag = 251
let string_tag = 252
let double_tag = 253
let double_array_tag = 254
let custom_tag = 255
let final_tag = custom_tag
sim642 commented 8 months ago

no_scan_tag is abstract_tag, so that's handled. According to this, everything smaller than no_scan_tag should be a properly structured block that should be safe to inspect. Everything above is handled already, so we should be all covered there.