Fix various cast errors #19802
Merged
Fix various cast errors #19802
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The store of NULL in the address pointer is unnecessary, as a few lines down this store occurs if fieldInfoCanBeUsed is false and the address pointer is not NULL. Removing this store has the effect of eliminating a clang warning; clang is wary of converting the `U_32` value 0 to a `void *` value, which is what was happening on this line. Signed-off-by: Christian Despres <[email protected]>
The check here is supposed to guard against the UB of casting a floating-point value outside the int32_t range to an int32_t. The value 0x7fffffff (decimal 2147483647) is not exactly representable as a 32-bit float; both gcc and clang convert it to the float value 2147483648.0 (one more than the literal). The added explicit `(float)` cast eliminates a clang warning about the issue of inexact representation. The check is also changed to `>=`, as in light of the above the previous code would fail if `weight` were ever exactly the floating-point value 2147483648.0. Signed-off-by: Christian Despres <[email protected]>
|
Attn @mpirvu. |
mpirvu
reviewed
Jul 3, 2024
| @@ -2570,7 +2570,6 @@ TR_ResolvedRelocatableJ9JITServerMethod::staticAttributes(TR::Compilation * comp | |||
|
|
|||
| if (!resolveField) | |||
There was a problem hiding this comment.
It's possible that resolvedField is false and fieldInfoCanBeUsed is true. Under these circumstances we would have written a NULL into *address which no longer happens in the new code.
There was a problem hiding this comment.
The fact that we write at *address without checking whether that address is not NULL is not ok.
There was a problem hiding this comment.
If resolvedInfo is false, it looked like we set fieldInfoCanBeUsed to false inside the if (!resolveField) block, so we'd end up execute the if (!fieldInfoCanBeUsed) block anyway. I could be misreading that.
|
jenkins test sanity zlinux,xlinuxjit,zlinuxjit,alinux64jit jdk17 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This store of
NULLinaddressinTR_ResolvedRelocatableJ9JITServerMethod::staticAttributesis unnecessary, as the same store occurs later on in that function iffieldInfoCanBeUsedis false andaddressis notNULL. It has been removed, which also eliminates a clang error - it is wary of converting a(U_32)0value to avoid *value.The
weightcheck inTR_MultipleCallTargetInliner::scaleSizeBasedOnBlockFrequencyis supposed to guard against the UB of casting a floating-point value outside the int32_t range to an int32_t. The value0x7fffffff(decimal 2147483647) is not exactly representable as a 32-bit float; both gcc and clang convert it to the float value 2147483648.0 (one more than the literal). The added explicit(float)cast eliminates a clang warning about the issue of inexact representation.The check is also changed to
>=, as in light of the above the previous code would fail ifweightwere ever exactly the floating-point value 2147483648.0.