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