[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)

[Kamlesh Kumar via lldb-commits]

kamleshbhalui wrote:

> > > This uses `DataExtractor::GetMaxU64` which already does this under the hood. What does this do that isn't already being done? It may help if you add a test case to show what you are trying to fix.
> > 
> > 
> > @clayborg @bulbazord The problem with GetMaxU64 is that it always returns uint64_t even though actual type was uint32_t, so when byteswap is performed it becomes invalid integer, to fixed this we need to store correct bitwidth not higher. i.e. Suppose there is actual 32 bit integer i.e. 0xffffffff `GetMaxU64` will return 0x00000000ffffffff (prmoted to uint64_t from uint32_t) and when performing byteswap on this it becomes 0xffffffff00000000 which is invalid.
> 
> So you are saying someone is taking the resulting Scalar and trying to byteswap it? Errors can still happen then in this class for int8_t/uint8_t and int16_t/uint16_t as there are no overloads for these in the `Scalar` class. We only currently have:
> 
> ```
>   Scalar(int v);
>   Scalar(unsigned int v);
>   Scalar(long v);
>   Scalar(unsigned long v);
>   Scalar(long long v);
>   Scalar(unsigned long long v);
> ```
> 
> It would suggest if we are going to make a requirement that Scalar will hold the correct bit widths, then we need to stop using "int" and "long" and switch to using the `int*_t` and `uint*_t` typedefs to make things clear:
> 
> ```
>   Scalar(int8_t v);
>   Scalar(int16_t v);
>   Scalar(int32_t v);
>   Scalar(int64_t v);
>   Scalar(uint8_t v);
>   Scalar(uint16_t v);
>   Scalar(uint32_t v);
>   Scalar(uint64_t v);
> ```
> 
> Then we have all of the bit widths handled correctly.

@clayborg I agree, we should use suggested type to fix it for all.

https://github.com/llvm/llvm-project/pull/81451