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

wip: stacktrace: enable method signature printing for inlined frames #53925

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Apr 2, 2024

This is an attempt to revive #41099 using the newly added invert linetable format (#52415).
It's a work in progress, but I've opened this PR to spark a discussion on whether I'm moving in the right direction with this approach. Currently, I've tweaked jl_lookup_code_address to give back a CodeInstance containing debuginfo rather than the MethodInstance for the outermost frame. This info is then used to find the MethodInstance for inlined inner frames from the Julia side.

julia> function f1(a)
           x = a
           return begin
               @inline f2(x)
           end
       end;

julia> function f2(a)
           x = a
           return @inline sin(x)
       end;

julia> f1(Inf)
ERROR: DomainError with Inf:
sin(x) is only defined for finite x.
Stacktrace:
diff --git a/old b/new
index f913ab20c3..215037aa56 100644
--- a/old
+++ b/new
@@ -1,10 +1,10 @@
  [1] sin_domain_error(x::Float64)
    @ Base.Math ./special/trig.jl:28
- [2] sin
-   @ ./special/trig.jl:39 [inlined]
- [3] f2
-   @ ./REPL[2]:3 [inlined]
+ [2] sin(x::Float64)
+   @ Base.Math ./special/trig.jl:39 [inlined]
+ [3] f2(a::Float64)
+   @ Main ./REPL[2]:3 [inlined]
  [4] f1(a::Float64)
    @ Main ./REPL[1]:4
  [5] top-level scope
    @ REPL[3]:1

@aviatesk aviatesk requested a review from vtjnash April 2, 2024 18:16
end
linfo = linfo.def
elseif debuginfo isa Core.DebugInfo
# TODO lookup a program counter `pc` at the parent frame and use `buildLineInfoNode(debuginfo, nothing, pc)`
Copy link
Member Author

Choose a reason for hiding this comment

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

One clear issue of the current implementation is we can't pinpoint the program counter for the outermost frame, making buildLineInfo unusable and causing lookups to fail except in the simplest cases. Is there a way, within lookup(pointer::Ptr{Cvoid}), to get a pc to feed into Base.IRShow.buildLineInfo(codeinst.debuginfo, nothing, pc)?

@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2024

We could hijack the dwarf column to be the pc, or just entirely replace the line values with an index into our richer information stack

Base automatically changed from avi/minor-cleanup to master April 3, 2024 04:16
@aviatesk aviatesk force-pushed the avi/inlined-stacktrace branch from c88b8ee to 1ade7fd Compare April 3, 2024 05:09
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