Skip to content

debug info missing from Tracy #13315

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

Closed
andrewrk opened this issue Oct 26, 2022 · 11 comments · Fixed by #17688
Closed

debug info missing from Tracy #13315

andrewrk opened this issue Oct 26, 2022 · 11 comments · Fixed by #17688
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Milestone

Comments

@andrewrk
Copy link
Member

Zig Version: 0.10.0-dev.4583+875e98a57

image

To reproduce:

stage3/bin/zig build -p stage4-tracy -Dskip-install-lib-files  -Drelease -Dtracy=$HOME/Downloads/tracy -Dstatic-llvm --search-prefix ~/dev/zig-bootstrap/out/x86_64-linux-musl-baseline -Dtarget=native-native-musl

Next run tracy, press the connect button. Finally:

stage4-tracy/bin/zig build
@andrewrk andrewrk added frontend Tokenization, parsing, AstGen, Sema, and Liveness. downstream An issue with a third party project that uses Zig. backend-llvm The LLVM backend outputs an LLVM IR Module. labels Oct 26, 2022
@andrewrk andrewrk added this to the 0.11.0 milestone Oct 26, 2022
@andrewrk
Copy link
Member Author

Downstream issue filed: wolfpld/tracy#482

@kubkon
Copy link
Member

kubkon commented Oct 27, 2022

Even worse so, this almost immediately crashes on macOS while some debug info is there.

@kubkon
Copy link
Member

kubkon commented Oct 27, 2022

This appears to be an issue with stage2 compiler. I've hooked up the same very bindings in https://github.com/kubkon/zld/tree/tracy-integration and tracy crashes with the same segfault as reported here when zld is built with stage2 compiler, but is fine when built with stage1 compiler (via -fstage1). I will try to investigate a little more since I have everything in place to debug it.

@kubkon kubkon removed the downstream An issue with a third party project that uses Zig. label Oct 27, 2022
@jamii
Copy link

jamii commented Nov 29, 2022

I hit possibly the same crash in tigerbeetle with zig 0.9.1. As a workaround, I found that the alloc api seems to work fine:

        ___tracy_emit_zone_begin_alloc(___tracy_alloc_srcloc(
            src.line,
            src.file.ptr,
            src.file.len,
            src.fn_name.ptr,
            src.fn_name.len,
        ), 1);

@leecannon
Copy link
Contributor

I have been experiencing this issue with all zig projects since switching to stage2.

@Techatrix found that if you don't use .line = src.line but instead hardcode it to .line = 1 it works (at least for the projects i've tested)

@leecannon
Copy link
Contributor

leecannon commented Mar 14, 2023

TLDR: tracy assumes the source location is static and it can read it at anytime, but we are passing a pointer to the stack

I have done a bit of investigation and found out that the source location must survive (the tracy manual even spells this out in section 3.4 Marking Zones in the important box at the end of the section), so it cannot be on the stack, this is why ___tracy_alloc_srcloc as mentioned by @jamii works as it allocates the source location on the heap and never frees it.

According to the dissasembly, if you do .line = 1 then the struct is in rodata so will survive and if you do .line = src.line the struct is constructed on the stack and becomes invalid almost immediately.

If you try to force it to be comptime known:

const location = comptime ___tracy_source_location_data{
    .name = null,
    .function = src.fn_name.ptr,
    .file = src.file.ptr,
    .line = src.line,
    .color = 0,
};

An error message stating that src.line is a runtime value is shown:

src/tracy.zig:67:20: error: cannot store runtime value in compile time variable
        .line = src.line,

@andrewrk is it intended that src.line on the parameter comptime src: std.builtin.SourceLocation is not comptime known?

@andrewrk andrewrk added the regression It worked in a previous version of Zig, but stopped working. label Mar 14, 2023
@andrewrk
Copy link
Member Author

No it's not intended. This is a regression.

@Vexu
Copy link
Member

Vexu commented Mar 15, 2023

src.line is not comptime-known to avoid breaking incremental compilation #12016 (comment)

@tau-dev
Copy link
Contributor

tau-dev commented May 5, 2023

You can even work around this without calling ___tracy_alloc_srcloc:

pub inline fn trace(comptime src: std.builtin.SourceLocation) Ctx {
    if (!enable) return .{};

    const statics = struct {
        var call: ___tracy_source_location_data = .{
            .name = null,
            .function = undefined,
            .file = undefined,
            .line = undefined,
            .color = 0,
        };
    };

    statics.call.function = src.fn_name.ptr;
    statics.call.file = src.file.ptr;
    statics.call.line = src.line;

    if (enable_callstack)
        return ___tracy_emit_zone_begin_callstack(&statics.call, callstack_depth, 1)
    else
        return ___tracy_emit_zone_begin(&statics.call, 1);
}

Since the function is inline, a static variable is created for each callsite.

Also, if @src() is runtime-known, shouldn't it be a compilation error to pass it to the comptime parameter? Is this a known bug, or did I misunderstand comptimeness again? :P

@mlugg
Copy link
Member

mlugg commented Jun 14, 2023

Just posting this context from Andrew on Discord:

one note on perf though, I plan to fix #13315 soon so that we have that additional tracing tool at our disposal

idea is that line numbers become comptime-known, but lazy values, and make it a compile error for trying to branch on them at compile-time

This treatment should also be given to the column, since incremental compilation could break in the same way there on code not formatted with zig fmt.

@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jul 24, 2023
@andrewrk
Copy link
Member Author

Regressed in 89cef9f.

andrewrk added a commit that referenced this issue Oct 24, 2023
andrewrk added a commit that referenced this issue Oct 24, 2023
When the code is written this way, you get a compile error if the
pointer given to Tracy does not have a static lifetime.

This would have caught the regression in #13315.
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Oct 24, 2023
mlugg pushed a commit to mlugg/zig that referenced this issue Oct 24, 2023
mlugg pushed a commit to mlugg/zig that referenced this issue Oct 24, 2023
When the code is written this way, you get a compile error if the
pointer given to Tracy does not have a static lifetime.

This would have caught the regression in ziglang#13315.
TUSF pushed a commit to TUSF/zig that referenced this issue May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants