On Wed, 16 Aug 2023 12:22:15 GMT, Christian Hagedorn <chaged...@openjdk.org> wrote:
>> src/hotspot/share/utilities/elfFile.cpp line 1426: >> >>> 1424: return false; >>> 1425: } >>> 1426: _state->_discriminator = static_cast<uint32_t>(discriminator); >> >> @chhagedorn These fields are declared as 32 bits but are read out of the >> dwarf info as 64 bits. Should they be 64 bits? I just made them 64 bits. > > 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 ? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1295823096
