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

changes to adapt to compressed line table format #606

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

vtjnash
Copy link
Contributor

@vtjnash vtjnash commented Jan 3, 2024

This contains some of the updates needed to keep working with the optimized debuginfo format (JuliaLang/julia#52415). This is a draft, since someone needs integrate this to maintain support for old versions, where the old code is mostly marked here with my initials (jwn) and my initials should be removed entirely and instead the implementation in those places fixed.

@aviatesk aviatesk force-pushed the jn/invert-line-table branch 2 times, most recently from 30bc00f to 3d7aec7 Compare January 5, 2024 12:42
@vtjnash vtjnash marked this pull request as draft January 5, 2024 16:28
@vtjnash vtjnash force-pushed the jn/invert-line-table branch from 3d7aec7 to 4c194db Compare February 23, 2024 21:33
@vtjnash vtjnash force-pushed the jn/invert-line-table branch from 4c194db to 9a1e6dc Compare March 7, 2024 21:13
@vtjnash vtjnash marked this pull request as ready for review March 7, 2024 21:15
@vtjnash vtjnash changed the title WIP changes to adapt to compressed line table format changes to adapt to compressed line table format Mar 7, 2024
@vtjnash vtjnash force-pushed the jn/invert-line-table branch 3 times, most recently from 9bef527 to cb420bb Compare March 11, 2024 17:49
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 33.92857% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 84.11%. Comparing base (31253a0) to head (ebc743a).

Files Patch % Lines
src/utils.jl 42.22% 26 Missing ⚠️
src/interpret.jl 0.00% 10 Missing ⚠️
src/types.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #606      +/-   ##
==========================================
- Coverage   85.13%   84.11%   -1.02%     
==========================================
  Files          12       12              
  Lines        2610     2644      +34     
==========================================
+ Hits         2222     2224       +2     
- Misses        388      420      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aviatesk
Copy link
Member

It looks like this "optimization" introduces a very bad interaction with the new lineinfo format:

# TODO: because of builtins.jl, for CodeInfos like
# %1 = Core.apply_type
# %2 = (%1)(args...)
# it would be best to *not* resolve the GlobalRef at %1
## Replace GlobalRefs with QuoteNodes
for (i, stmt) in enumerate(code.code)
if isa(stmt, GlobalRef)
code.code[i] = lookup_global_ref(stmt)
elseif isa(stmt, Expr)
if stmt.head === :call && stmt.args[1] === :cglobal # cglobal requires literals
continue
else
lookup_global_refs!(stmt)
code.code[i] = lookup_getproperties(code.code, stmt)
end
end
end

julia> using JuliaInterpreter

julia> fnone1() = nothing;

julia> let m = only(methods(fnone1))
           JuliaInterpreter.FrameCode(m, JuliaInterpreter.get_source(m); optimize=true).src.debuginfo
       end
Core.DebugInfo(Symbol("REPL[2]"), nothing, svec(), "\x01\0\0\0\0\0\0\0\0\0\0\0\x01\x01\0")

julia> fnone2() = nothing;

julia> let m = only(methods(fnone2))
           JuliaInterpreter.FrameCode(m, JuliaInterpreter.get_source(m); optimize=true).src.debuginfo
       end
Core.DebugInfo(Symbol("REPL[4]"), nothing, svec(), "")

@aviatesk aviatesk force-pushed the jn/invert-line-table branch 2 times, most recently from cb9b79a to 16b332c Compare March 21, 2024 13:54
src/utils.jl Outdated
@static if VERSION ≥ v"1.12.0-DEV.173"
# TODO: decode the linetable at this frame efficiently by reimplementing this here
# TODO: get the contextual name from the parent, rather than returning "n/a" (which breaks Cthulhu)
return Base.IRShow.buildLineInfoNode(lt, :var"n/a", i)[1] # ignore all inlining / macro expansion / etc :(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this commit -- do we now store some macro expansion context within debuginfo?

@aviatesk aviatesk force-pushed the jn/invert-line-table branch 3 times, most recently from 6a07c3e to fa51fb1 Compare March 25, 2024 16:51
@aviatesk aviatesk force-pushed the jn/invert-line-table branch from fa51fb1 to ebc743a Compare March 26, 2024 04:40
end
return lastline
end
function codelocs(arg, i::Integer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For statementnumbers to function in v1.12, there's a need for a tool that can rebuild the full codelocs object from Core.DebugInfo, similar to what we could do just by looking at src.codelocs previously. Or, we may have to overhaul the statementnumbers algorithm.

@aviatesk
Copy link
Member

This PR is obviously not finished yet, but I want to push ahead with it as is to avoid holding up progress on other packages in the ecosystem. We can tackle the specific issues that I've identified so far later on (likely when Jameson returns).

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.

2 participants