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

Add opaque value inspection #53

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Add opaque value inspection #53

merged 3 commits into from
Jan 5, 2024

Conversation

hackwaly
Copy link
Owner

@hackwaly hackwaly commented Oct 17, 2023

Close #52

TODO:

  • string value
  • int value
  • float value
  • block value
  • heuristic list value
  • test
image

@hackwaly hackwaly requested a review from sim642 October 17, 2023 03:57
@hackwaly hackwaly marked this pull request as draft October 17, 2023 03:57
@hackwaly hackwaly force-pushed the feat/inspect-opaque branch 2 times, most recently from 2590f58 to a53156d Compare October 17, 2023 07:46
@hackwaly
Copy link
Owner Author

Realuse feedback: sometimes segfault occurs

@sim642 sim642 added the enhancement New feature or request label Nov 5, 2023
@hackwaly
Copy link
Owner Author

hackwaly commented Jan 5, 2024

Realuse feedback: sometimes segfault occurs

fixed.

@hackwaly hackwaly marked this pull request as ready for review January 5, 2024 06:42
@sim642
Copy link
Collaborator

sim642 commented Jan 5, 2024

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
Copy link
Owner Author

hackwaly commented Jan 5, 2024

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
Copy link
Collaborator

sim642 commented Jan 5, 2024

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
Copy link
Owner Author

hackwaly commented Jan 5, 2024

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

@sim642
Copy link
Collaborator

sim642 commented Jan 5, 2024

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
Copy link
Owner Author

hackwaly commented Jan 5, 2024

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
Copy link
Collaborator

sim642 commented Jan 5, 2024

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.

@sim642 sim642 merged commit d4a95a4 into master Jan 5, 2024
10 checks passed
sim642 added a commit to sim642/opam-repository that referenced this pull request Jan 5, 2024
CHANGES:

### Added

* Add opaque value inspection using runtime representation (hackwaly/ocamlearlybird#9, hackwaly/ocamlearlybird#52, hackwaly/ocamlearlybird#53).
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

### Added

* Add opaque value inspection using runtime representation (hackwaly/ocamlearlybird#9, hackwaly/ocamlearlybird#52, hackwaly/ocamlearlybird#53).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use runtime representation to display <<opaque>> value
2 participants