librustc_trans: function parameters should be annotated with zeroext/signext

[antonblanchard]

type_of -> Type::uint_from_ty/Type::int_from_ty converts to LLVM types which have no sign. This is then passed to the architecture specific code in compute_abi_info().

PowerPC needs to know whether to sign or zero extend function call parameters, but it doesn't have enough information in the LLVM types.

[Aatch]

I don't think it's compute_abi_info() that needs it really, it's just that we should be adding zeroext or signext parameter attributes where appropriate. Looking at the output of clang it always adds them for types with a size less than 32-bits.

I'm curious if you have a PowerPC-specific example, as we don't add these for other platforms and it hasn't caused a problem there. Does PowerPC do something different that means it matters more here?

[antonblanchard]

This came up while reviewing #30776. I haven't seen a specific issue, but @eefriedman pointed out that I wasn't modelling the PowerPC64 ABI completely. The test case in C was:

void f(int); void g(unsigned long long x) { f(x); }

I'm not sure why x86_64 isn't annotating the LLVM IR with signext here.

[Aatch]

Well we're not annotating the parameters properly for any platform, which at least suggests that is relatively low priority in terms of actual problems caused. The reason being that most of the time you're calling out to C code that will itself only use the parts of the register for the size of the appropriate type. What I mean is that it doesn't matter if the value is sign- or zero-extended if code never actually relies on the extended portion of the value.

Anyway, I think the PowerPC aspect here is a bit of a red herring and we should be annotating all parameters that need the annotations,

[antonblanchard]

Agreed, changing the title to match.

[Aatch]

Ah, I just figured out/found out why PowerPC is a problem specifically. PowerPC apparently doesn't support 8-bit or 16-bit operations natively and therefore you can't tell it to just add (for example) together to lowest byte/word in each register.

I'm surprised this hasn't caused any issues with calling out to C code, a quick test suggests that it could cause issues if you have a C function accepting signed bytes, Rust will just zero-extend them to the register size. This is on x86 too, so it's definitely a not a PPC-specific issue, and it actually much more serious than I first though.

[nikomatsakis]

triage: P-medium

Good catch reporting this bug! This should definitely be fixed, but PowerPC is not a "tier 1" platform, so it's not a high priority (drop everything) sort of bug.

[Aatch]

@nikomatsakis this does affect other platforms though. Clang will assume that char function arguments have been sign-extended when generating x86 code, but we won't do so which means it could cause an issue.

I'm not disagreeing with the priority, I think medium is fine, just clarifying that this is a general FFI issue, not just a PPC issue.

[nikomatsakis]

thanks for the clarification

On Mon, Jan 25, 2016 at 08:37:23PM -0800, James Miller wrote:

@nikomatsakis this does affect other platforms though. Clang will assume that char function arguments have been sign-extended when generating x86 code, but we won't do so which means it could cause an issue.

I'm not disagreeing with the priority, I think medium is fine, just clarifying that this is a general FFI issue, not just a PPC issue.

---

Reply to this email directly or view it on GitHub:
#30841 (comment)