hrydgard / minitrace

Simple C/C++ library for producing JSON traces suitable for Chrome's built-in trace viewer (about:tracing).
MIT License
373 stars 64 forks source link

Segfault in mtr_flush #9

Closed DethRaid closed 5 years ago

DethRaid commented 6 years ago

I'm trying to use minitrace to profile my application, but minitrace seems to have other ideas.

I'm making a renderer. My renderer's shutdown process is... complicated, so I'm trying to flush minitrace's profiler data every few frames. The first frame executes without trouble, but on the second frame I get a segfault at line 249 in minitrace.c. It looks like strlen is segfaulting - which the internet suggests could be because the string raw->a_str isn't null-terminated?

hrydgard commented 5 years ago

I'm wondering if this might have been solved by #12?

DethRaid commented 5 years ago

I believe that #12 does fix my issue, however I've been hacking on my renderer a lot and there's lots of problems that prevent me from really testing it. Imma keep using minitrace and let you know if I run into any more problems

DethRaid commented 5 years ago

Update: I fixed the other issues and I'm still getting a segfault, on line 255 of minitrace.c. Looks like the same one I had earlier

DethRaid commented 5 years ago

For reference, https://github.com/NovaMods/nova-renderer/blob/master/src/nova_renderer.cpp is how I'm currently using Minitrace (everything is in that file)

hrydgard commented 5 years ago

Unless strdup is defined to something really bizarre in your codebase, I don't see how we could get a crash at that line. If you have nne event of type MTR_ARG_TYPE_STRING_COPY then the a_str would have been strdup'd and would be a nice null-terminated string.

DethRaid commented 5 years ago

Do you ever reset the data in the internal buffer?

I'm pouring over minitrace.c as closely as I can and I don't see anywhere the data in the buffer is reset

Theory: at the beginning of my program I have

MTR_META_PROCESS_NAME("NovaRenderer");
MTR_META_THREAD_NAME("Main");

which generate two events with type MTR_ARG_TYPE_STRING_COPY

The first frame, nothing terribly interesting happens. I have a few MTR_SCOPE sprinkled throughout my code. That macro creates a MTRScopedTrace object which gets destructed at the end of my function, The destructor calls internal_mtr_raw_event which is totally great, but - and I think this is the key - internal_mtr_raw_event _doesn't change raw_event_t::arg_type

The buffer gets flushed each frame, because even though I've changed my code to not need to do that I want to fix this bug. The flush sets count to 0 without clearing any of the data in the buffer

Perhaps you see where I'm going. The second frame, at least one of my MTRScopedTrace objects' destructors writes to a raw_event_t which was previously used for either MTR_META_PROCESS_NAME or MTR_META_THREAD_NAME - so the raw_event_t's arg_type is still set to MTR_ARG_TYPE_STRING_COPY, even though there's no strings here to copy. When the second mtr_flush call happens, it reads the raw_event_t commands with arg_type of MTR_ARG_TYPE_STRING_COPY - but their a_str is no longer a valid pointer, causing my crash

Solution: set raw_event_t::arg_type to MTR_EVENT_NONE in internal_mtr_raw_event

Expect a PR shortly

DethRaid commented 5 years ago

PR is accepted, closing this issue