-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix g++-14 warning on uninitialized copying #2157
Conversation
``` In function bool cuda::std::__4::__dispatch_memmove(_Up*, _Tp*, size_t) ... error: *(unsigned char*)(&privatized_decode_op[0]) may be used uninitialized [-Werror=maybe-uninitialized] ... *(unsigned char*)(&privatized_decode_op[0]) was declared here 1528 | PrivatizedDecodeOpT privatized_decode_op[NUM_ACTIVE_CHANNELS]{}; ```
434a3fd
to
43fce72
Compare
🟨 CI finished in 2h 43m: Pass: 99%/250 | Total: 1d 09h | Avg: 8m 00s | Max: 33m 06s | Hits: 98%/249175
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
Thrust | |
CUDA Experimental | |
pycuda |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | pycuda |
🏃 Runner counts (total jobs: 250)
# | Runner |
---|---|
178 | linux-amd64-cpu16 |
41 | linux-amd64-gpu-v100-latest-1 |
16 | linux-arm64-cpu16 |
15 | windows-amd64-cpu16 |
// GCC 14 rightfully warns that when a value-initialized array of this struct is copied using memcpy, uninitialized | ||
// bytes may be accessed. To avoid this, we add a dummy member, so value initialization actually initializes the memory. | ||
#if defined(_CCCL_COMPILER_GCC) && __GNUC__ == 14 | ||
char dummy; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Would it suffice to provide a user defined default constructor that does nothing? That should technically zero out struct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not solve the issue and I get the same error message. That is, adding a constructor PassThruTransform() {}
does not prevent the warning.
I checked the code generated by |
🟩 CI finished in 12h 31m: Pass: 100%/250 | Total: 1d 09h | Avg: 8m 02s | Max: 33m 06s | Hits: 98%/250042
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
Thrust | |
CUDA Experimental | |
pycuda |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | pycuda |
🏃 Runner counts (total jobs: 250)
# | Runner |
---|---|
178 | linux-amd64-cpu16 |
41 | linux-amd64-gpu-v100-latest-1 |
16 | linux-arm64-cpu16 |
15 | windows-amd64-cpu16 |
@@ -844,6 +845,12 @@ public: | |||
// Pass-through bin transform operator | |||
struct PassThruTransform | |||
{ | |||
// GCC 14 rightfully warns that when a value-initialized array of this struct is copied using memcpy, uninitialized | |||
// bytes may be accessed. To avoid this, we add a dummy member, so value initialization actually initializes the memory. | |||
#if defined(_CCCL_COMPILER_GCC) && __GNUC__ == 14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be >=
rather than ==
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could. But I would like to run into this issue again with GCC 15, since there is a chance they fix their bug :)
@@ -414,6 +414,7 @@ struct dispatch_histogram | |||
d_output_histograms, d_output_histograms + NUM_ACTIVE_CHANNELS, d_output_histograms_wrapper.begin()); | |||
::cuda::std::copy( | |||
typedAllocations, typedAllocations + NUM_ACTIVE_CHANNELS, d_privatized_histograms_wrapper.begin()); | |||
// TODO(bgruber): we can probably skip copying the function objects when they are empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this would be a nicer fix, but as a hotfix what you have here is fine.
``` In function bool cuda::std::__4::__dispatch_memmove(_Up*, _Tp*, size_t) ... error: *(unsigned char*)(&privatized_decode_op[0]) may be used uninitialized [-Werror=maybe-uninitialized] ... *(unsigned char*)(&privatized_decode_op[0]) was declared here 1528 | PrivatizedDecodeOpT privatized_decode_op[NUM_ACTIVE_CHANNELS]{}; ```
``` In function bool cuda::std::__4::__dispatch_memmove(_Up*, _Tp*, size_t) ... error: *(unsigned char*)(&privatized_decode_op[0]) may be used uninitialized [-Werror=maybe-uninitialized] ... *(unsigned char*)(&privatized_decode_op[0]) was declared here 1528 | PrivatizedDecodeOpT privatized_decode_op[NUM_ACTIVE_CHANNELS]{}; ```
This PR fixes a warning with g++-14 (and nvbug 4748765). Here is the summary:
PrivatizedDecodeOpT
isPassThruTransform
in this context, which is a struct of no data members. Therefore, it occupies 1 byte of storage, but is technically empty. The value initialization seems to not touch any bytes, so the latermemmove
complains it is transferring unwritten bytes.I cannot suppress this warning, since it occurs at the locus of
__dispatch_memmove
, so I went for adding a dummychar
data member toPassThruTransform
. This way (I assume) value initializing does zero the memory beforememmove
touches it.