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

Fix signedness issue in the DWARF line parser on ARM64 #17031

Merged
merged 6 commits into from
Jun 12, 2020

Conversation

HoundThe
Copy link
Contributor

@HoundThe HoundThe commented Jun 9, 2020

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've added tests that prove my fix is effective or that my feature works (if possible)
    (tests are already there)

I am abusing PR so I can run tests on ARM to check if I found the error for #15688
I think it might be because signedness of char is implementation defined and the line_base is supposed to be -5, but instead it's 251 which is -5 on 8bit 2's complement

@HoundThe
Copy link
Contributor Author

HoundThe commented Jun 9, 2020

Seems that it worked, it was the signedness issue
https://travis-ci.com/github/radareorg/radare2/builds/170593567

@HoundThe HoundThe changed the title Implementation defined signedness fix DWARF - Implementation defined signedness fix Jun 9, 2020
@HoundThe HoundThe added the DWARF DWARF symbols and types debug information label Jun 9, 2020
@trufae
Copy link
Collaborator

trufae commented Jun 9, 2020

scary that a char can be unsigned somewhere :S good finding!

@XVilka
Copy link
Contributor

XVilka commented Jun 10, 2020

Maybe use the usual int instead of signed char (convert upon reading)?

@HoundThe
Copy link
Contributor Author

Sure, why not

libr/bin/dwarf.c Outdated Show resolved Hide resolved
@XVilka XVilka added this to the 4.5.0 - Organized Chaos milestone Jun 11, 2020
@XVilka XVilka self-requested a review June 11, 2020 04:31
@trufae trufae changed the title DWARF - Implementation defined signedness fix Fix signedness issue in the DWARF line parser on ARM64 Jun 11, 2020
@XVilka XVilka merged commit f298057 into radareorg:master Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DWARF DWARF symbols and types debug information ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants