[clang] [compiler-rt] [ubsan] Display correct runtime messages for negative _BitInt (PR #96240)
Björn Pettersson via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 29 12:18:01 PDT 2024
================
@@ -103,6 +103,13 @@ class TypeDescriptor {
/// representation is that of bitcasting the floating-point value to an
/// integer type.
TK_Float = 0x0001,
+ /// An _BitInt(N) type. Lowest bit is 1 for a signed value, 0 for an
+ /// unsigned value. Remaining bits are log_2(bit_width). The value
----------------
bjope wrote:
The new thing in this patch is that the size of the _BitInt type being described is stored after the type string. It can be read using `getIntegerBitCount()`.
Thus, to get the size of the type being described one should no longer use `getIntegerBitWidth()` but instead `getIntegerBitCount()`. The latter would give correct result for both TK_Integer and TK_BitInt, while the former doesn't work for BitInt. IMHO it had been better to keep the name `getIntegerBitWidth()` for this purpose. Now I figure most/all old uses of `getIntegerBitWidth()` actually should be replaced with calls to `getIntegerBitCount()`, but that has not happened as far as I can tell.
The new behavior of `getIntegerBitWidth()` is merely to indicate the size used to store the Value being represented when it does not fit inside the ValueHandle. So there shouldn't really be any need to use that function outside `Value::getSIntValue()` (assuming that `getIntegerBitCount()` would know how to get the size of TK_Integer via the TypeInfo field by itself).
And unfortunatly there isn't much information about how those non-inlined values are store. The `Value::getSIntValue()` functions is just reading an s64 or s128 from memory (and that is what it traditionally has been doing for TK_Integer). But then I think the IR emitted by clang should cast the value to i64/i128 instead of doing
```
RawAddress Ptr = CreateDefaultAlignTempAlloca(V->getType());
Builder.CreateStore(V, Ptr);
```
Now we for example get things like `store i66 %x, ptr %tmp` which is storing a non byte-sized value to memory, which then is loaded by the runtime as a 128-bit value.
Worth noting is that after commit 9ad72df55cb74b2919327 (#91364) the frontend emits byte-sized loads/stores for _BitInt. So I really think the store emitted by CodeGenFunction::EmitCheckValue for the BitInt values should zero/sign extended similarly (as if they were casted to "s64" or "s128" before being stored). That would at least make the code a bit more consistent (even though the question about supporting values larger than 128-bits would remain).
https://github.com/llvm/llvm-project/pull/96240
More information about the cfe-commits
mailing list