Clang's isSignedCharDefault() is incorrect for multiple architectures

[arichardson]

While debugging a signed/unsigned char issue I started looking at the list of triples that use unsigned chars by default and it appears the logic in Clang does not match the ABI documents for some architectures. I noticed some issues while comparing isSignedCharDefault() with the Rust list of systems with unsigned char: https://github.com/rust-lang/rust/blob/6503543d11583d1686d4989847b2afbec8d9fdba/library/core/src/ffi/mod.rs#L92

So far it appears s390x, csky, xtensa, msp430 are missing from isSignedCharDefault(), but there might be other omissions.

• csky: https://github.com/c-sky/csky-doc/blob/master/C-SKY_V2_CPU_Applications_Binary_Interface_Standards_Manual.pdf says CSKY should have unsigned char by default
• Misread the clang source, is already correct s390x: https://github.com/IBM/s390x-abi states the range of char is 0 … 2^n − 1, i.e. it uses unsigned char
• msp430: https://www.ti.com/lit/pdf/slaa534 states the char type is unsigned by default, GCC agrees
• xtensa: GCC says char is unsigned, not sure where the ABI document is.

I don't see a CSKY GCC on godbolt, but https://godbolt.org/z/qaaW18znY confirms that the other architectures diverge from GCC

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Alexander Richardson (arichardson)

While debugging a signed/unsigned char issue I started looking at the list of triples that use unsigned chars by default and it appears the logic in Clang does not match the ABI documents for some architectures. I noticed some issues while isSignedCharDefault() with the Rust list of systems with unsigned char: https://github.com/rust-lang/rust/blob/6503543d11583d1686d4989847b2afbec8d9fdba/library/core/src/ffi/mod.rs#L92

So far it appears, s390x and csky, xtensa and msp430 are missing from isSignedCharDefault(), but there might be other omissions.

• csky: https://github.com/c-sky/csky-doc/blob/master/C-SKY_V2_CPU_Applications_Binary_Interface_Standards_Manual.pdf says CSKY should have unsigned char by default
• s390x: https://github.com/IBM/s390x-abi states the range of char is 0 … 2^n − 1, i.e. it uses unsigned char
• msp430: https://www.ti.com/lit/pdf/slaa534 states the char type is unsigned by default, GCC agrees
• xtensa: GCC says char is unsigned, not sure where the ABI document is.

I don't see a CSKY GCC on godbolt, but https://godbolt.org/z/qaaW18znY confirms that the other architectures diverge from GCC

[shafik]

CC @erichkeane @AaronBallman

[AaronBallman]

Something to consider as we fix these: we may need to provide ABI tags so users can still get the old (incorrect) ABI as needed.

[arichardson]

Do we really need a compatibility fallback here? The sign of char rarely matters for code generation and I believe none of the affected targets are "tier 1"? Affected projects can always use -f(no)-signed-char.

[AaronBallman]

Do we really need a compatibility fallback here?

Because this impacts the answer you get back from type traits like is_signed_v, and those tend to get used when specializing templates, the ABI impact can be surprisingly wide.

The sign of char rarely matters for code generation and I believe none of the affected targets are "tier 1"? Affected projects can always use -f(no)-signed-char.

That's a good point I hadn't considered, but it seems somewhat unclean to me. That gets us to "to get the Clang 19 ABI, pass -fclang-abi-compat=19 but you may also need to set -f(no)-signed-char as well" which isn't great.

[arichardson]

The template specialization is a good point, I had not considered that.

I think the main question is whether the affected targets have a real binary compatibility story or tend to be more of an embedded "recompile the whole system" target.

• MSP430 is a microcontroller, so I'd imagine limited binary compatibility concerns.
• I don't know anything about CSKY but it is 32-bit embedded with Linux support, so that may have stronger requirements.
• I also don't know much about XTensa, but it also appears to be used for embedded architectures.

We could forward args from RenderCharacterOptions to isSignedCharDefault add a check for OPT_fclang_abi_compat_EQ inside this function, but that would require some duplication unless we move this logic from the Driver to actual per-target LangOptions overrides.

Arguably the latter would be cleaner than handling this in the Driver (and there is already a FIXME). This would allow test/Preprocessor to check for the value of __CHAR_UNSIGNED__ instead of writing Driver tests as I did in #115967 and #115964. But I don't think I will be able find the time to do this larger refactoring any time soon.