Viladoman / CompileScore

Tools for profiling and visualizing C++ build times.
MIT License
460 stars 19 forks source link

Crash during extraction with MSVC #11

Closed GenuineAster closed 3 years ago

GenuineAster commented 3 years ago

This may be related to https://github.com/Viladoman/CompileScore/issues/10

I get a crash when stopping recording with MSVC, I debugged it and it crashes here: https://github.com/Viladoman/CompileScore/blob/master/DataExtractor/src/Extractors/MSVCScore.cpp#L45

Because both compilerPass.OutputObjectPath() and compilerPass.InputSourcePath() are nullptr. I am not well-versed enough in all this to tell you why! I can get the program to complete execution by returning "[unknown]" if those are nullptr but then most units end up not having a name.

We are generating a VS2019 solution with CMake with MSVC as compiler. Let me know if I can provide more info!

Viladoman commented 3 years ago

Thanks for having a look and letting me know! Super appreciated!

It might, as you point out, be the same issue. Having those nullptr is not great. As you said guarding against it will be a good practice, but it will be nicer if we can get the actual names differently when this fails.

Can you replicate it with a small sample code that you could share with me? Or it only happens on a big complicated project? This way I can explore ways to extract the proper information.

I will add the guards or feel free to pull request your change to at least add stability.

GenuineAster commented 3 years ago

I tried only building small projects in our solution to replicate the issue but those worked fine, it only happens when I build everything. It could be related to size. I can try to pinpoint if a particular project of ours is causing this, and if I have time I will try to make a minimal repro case over the weekend.. But just a warning that I'm terrible at following up on these things :-)

This is a really cool project though! I have countless personal workflows for improving compile times but this project may very well be what motivates my team to do it themselves too!

Viladoman commented 3 years ago

Thanks for the kind words! That would be awesome! I would really appreciate it. I personally use Compile Score in quite massive codebases without crashing. I bet there is something special exposing this Extractor flaw. The good news are that we have a lead now.

GenuineAster commented 3 years ago

Okay, I managed to narrow it down to using the /MP flag (multi-processor compilation) with MSVC. A small repro case is here: https://gist.github.com/GenuineAster/efde162c6d805bee36f4a23b2cdba812

If you comment out the add_compile_options line (and build in a clean directory), then ScoreDataExtractor runs fine.

Viladoman commented 3 years ago

Finally got some time to look into this.

I reached out to the Build Insights SDK author and got no answer so far. We needed a solution if temporary, while those paths are not present in /MP. I decided to do some assumptions, so hopefully the data looks good (At least It did in my limited tests).

Now it should not crash and it should try to figure it out in a FIFO way which backend goes with which frontend (when no reference is present). Thankfully the first include in a frontend pass with MSVC contains the file itself, so I could extract the actual TU names.

tl:dr. I would appreciate if you could try version 1.5.6 and validate everything works as expected.

Thanks for your investigations and help so far.