On Wed, 16 Aug 2023 12:30:18 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> According to the DWARF 4 spec, the discriminator is an unsigned integer
>> (i.e. 32 bits). Also, the other fields like `isa or `column` are unsigned
>> integer. I think we would be good to keep them as unsigned integer to follow
>> the spec.
>>
>> `read_uleb128()` reads up to 8 bytes and just returns the result in a
>> `uint64_t`. But sometimes, it should only read 1, 2, or 4 bytes. In that
>> case, we additionally specify `check_size` to ensure that we can safely cast
>> it. I guess we could change these calls to `read_uleb128()` to return a more
>> precise type instead of relying on `check_size` passed in by a caller +
>> casting the result in the caller.
>
> When we read the 64 bit values in the code, the values are 64 bits though.
> So static_cast<> to narrow the result is more correct ?
This code:
uint64_t isa;
if (!_reader.read_uleb128(&isa, 4)) {
// isa register is 4 bytes wide.
return false;
}
_state->_isa = isa; // only save 4 bytes
returns 64 bit of information into isa from this read_uleb128 call, even though
it passes 4.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1295827701
