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

bugfix: don't crash when c++filt is not available in the toolchain #24

Conversation

sarfata
Copy link
Collaborator

@sarfata sarfata commented Oct 7, 2019

The PIC xc16 toolchain does not include any c++ tools and puncover crashed while trying to open the elf file. This fixes it.

@HBehrens
Copy link
Owner

HBehrens commented May 5, 2020

This one should still work once rebased and a unit-test exists, right @sarfata ?

@sarfata
Copy link
Collaborator Author

sarfata commented May 6, 2020

Yes this should still work. This was just blocked by CI issues.

@HBehrens
Copy link
Owner

HBehrens commented May 7, 2020

Cool, are you willing to rebase + provide a test that shows the behavior?

The PIC xc16 toolchain does not include any c++ tools and puncover
crashed while trying to open the elf file. This fixes it.
@sarfata sarfata force-pushed the bugfix/dont-crash-when-cppfilt-not-available branch from d810c45 to 35805d8 Compare May 11, 2020 16:05
@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #24 into master will decrease coverage by 0.07%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   74.49%   74.42%   -0.08%     
==========================================
  Files          17       17              
  Lines        1345     1349       +4     
==========================================
+ Hits         1002     1004       +2     
- Misses        343      345       +2     
Impacted Files Coverage Δ
puncover/gcc_tools.py 65.62% <60.00%> (-2.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 706dcc5...35805d8. Read the comment docs.

@sarfata
Copy link
Collaborator Author

sarfata commented May 11, 2020

I have rebased.

Making an integration test for this would require the xc16 toolchain from Microchip which has a complicated license (user has to download it from their website - cant bundle it). It also seems like a lot of efforts for a pretty simple change.

Overall, I am not sure this PR is really that useful since even with this fix, puncover did not work well with the elf files generated by this compiler (cf #26). For other toolchains built on GCC, I am guessing cppfilt will always be available.

Maybe we just close without merging and if someone picks up #26, this fix will be visible and can be bundled in.

@HBehrens
Copy link
Owner

Ok, will close then. Thank you for your work and perspective, @sarfata !

@HBehrens HBehrens closed this May 12, 2020
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