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

Add zeroext/signext attributes to C ABI functions where appropriate #31725

Closed
wants to merge 3 commits into from

Conversation

Aatch
Copy link
Contributor

@Aatch Aatch commented Feb 17, 2016

The C ABI on most platforms expects integers to be extended up to the appropriate size. This requires annotation in LLVM to indicate whether or not to do sign extension or zero extension. Foreign code may rely on the extension behaviour and for signed integers the value passed may be incorrect.

Fixes #30841 and #31315

We need to supply sext/zext attributes to LLVM to ensure that arguments
are extended to the appropriate width in the correct way.

This commit adds the necessary infrastructure for handling integer
widening and makes the x86-64 code produce the correct information.
Most platforms extend integers less than 32 bits, though not all.
@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -0,0 +1,4 @@
// ignore-license
int foo(char x) {
Copy link
Member

Choose a reason for hiding this comment

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

should this be signed char?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think char is signed already. At least, clang specifies that they're signext.

Copy link
Contributor

Choose a reason for hiding this comment

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

char is a distinct type from signed char and unsigned char and its signedness is implementation defined. And according to e.g. http://stackoverflow.com/questions/3728045/any-compiler-which-takes-char-as-unsigned char is in fact unsigned on ARM.

@arielb1
Copy link
Contributor

arielb1 commented Feb 17, 2016

r? @dotdash

@rust-highfive rust-highfive assigned dotdash and unassigned arielb1 Feb 17, 2016
@@ -0,0 +1,10 @@
-include ../tools.mk

all: $(call NATIVE_STATICLIB,cfoo)
Copy link
Member

Choose a reason for hiding this comment

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

This might be most easily added actually as part of src/rt/rust_test_helpers.c and then a standard run-pass test can be added. Helps avoid run-make at least!

@bors
Copy link
Contributor

bors commented Feb 19, 2016

☔ The latest upstream changes (presumably #31742) made this pull request unmergeable. Please resolve the merge conflicts.

@dotdash dotdash removed their assignment Mar 4, 2016
@alexcrichton alexcrichton self-assigned this Mar 6, 2016
@dotdash
Copy link
Contributor

dotdash commented Apr 4, 2016

Updated version of this is #32732

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.

7 participants