-
Notifications
You must be signed in to change notification settings - Fork 570
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 most warnings generated by Clang 5.0 on AArch64. #2965
Conversation
For the dr_extend_type_t enum type, Clang decided that it is unsigned. I am not sure if we can rely on Gcc to agree or how to ensure that. Change-Id: I1ab036338939541491a4f054ea034d9b0036599a
@@ -45,7 +45,7 @@ | |||
|
|||
int main() | |||
{ | |||
int pid; | |||
int64_t pid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the X86 inline assembly is not happy with this type. I will update that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be pid_t?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it back to int and limited the width in the AArch64 assembler.
core/arch/interp.c
Outdated
@@ -5711,6 +5713,7 @@ tracelist_add(dcontext_t *dcontext, instrlist_t *ilist, instr_t *where, instr_t | |||
return size; | |||
} | |||
|
|||
#ifdef X86 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We never did port traces to ARM but we should at some point, so this should not be x86-specific, or if you mark it that way at least make it clear this is temporary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -45,7 +45,7 @@ | |||
|
|||
int main() | |||
{ | |||
int pid; | |||
int64_t pid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be pid_t?
Change-Id: Ica446d32183aadd197b27f9da3faa7d942e50d98
For the dr_extend_type_t enum type, Clang decided that it is unsigned.
I am not sure if we can rely on Gcc to agree or how to ensure that.