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.

Sign up for GitHub

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

Jump to bottom

Incorrect handling of types causes a segfault on arm #1424

Merged
merged 1 commit into from
May 31, 2017
Merged

Incorrect handling of types causes a segfault on arm #1424

merged 1 commit into from
May 31, 2017

Conversation

wbkang
Copy link
Contributor

@wbkang wbkang commented May 30, 2017

On armv7l, char type is actually unsigned. So doing a switch on an unsigned value kindIndex with a negative case value actually does not work. My suggestion is to fix the type of kindIndex specifically to signed char. All unit tests pass if I make that change as well.

make units is passed.

Created an issue: #1423

I am contributing the code under the terms of the General Public License version 2 or any later version.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.416% when pulling 51ebf7d on wbkang:bugfix/unsignedswitch into 4b9aa92 on universal-ctags:master.

@masatake
Copy link
Member

On armv7l, char type is actually unsigned.
I was very suprised at this but you are right; I found the explanation in the K&R book in my hand.

valgrind is avaiable in your environment?

I would like you to run "make units VG=1". I hope valgrind reports something unwanted cses.

@masatake
Copy link
Member

I am contributing the code under the terms of the General Public License version 2 or any later version.

Thank you very much.

masatake
masatake approved these changes May 30, 2017
View reviewed changes
main/parse.h Outdated
@@ -148,7 +148,7 @@ extern bool doesLanguageAllowNullTag (const langType language);
extern bool doesLanguageRequestAutomaticFQTag (const langType language);
extern const char *getLanguageName (const langType language);
extern kindDefinition* getLanguageKindForLetter (const langType language, char kindLetter);
extern kindDefinition* getLanguageKind(const langType language, char kindIndex);
extern kindDefinition* getLanguageKind(const langType language, signed char kindIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask you to add comments explaining why "signed" is added here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done!

@wbkang
Incorrect handling of types causes a segfault on
4c423b7 
armv7l (e.g., Raspberry Pi)

universal-ctags#1423

make units is passed.
@wbkang wbkang force-pushed the bugfix/unsignedswitch branch from 51ebf7d to 4c423b7 Compare May 31, 2017 00:40
@wbkang
Copy link
Contributor Author

wbkang commented May 31, 2017

All test passed with valgrind on as well (it says #valgrind-error: 0). For the future reference, you have to comment out libarmmem.so in /etc/ld.so.preload (See: https://stackoverflow.com/a/42074785/2710) or else most valgrind tests will fail with error 139.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.416% when pulling 4c423b7 on wbkang:bugfix/unsignedswitch into 4146b4e on universal-ctags:master.

@masatake masatake merged commit 2064b8a into universal-ctags:master May 31, 2017
@masatake
Copy link
Member

Thank you.

@masatake masatake mentioned this pull request May 31, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masatake masatake masatake approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wbkang @coveralls @masatake