Fix -Wstringop-overflow warning #3440
Conversation
Backported from kernel patch [0]. I wasn't able to reproduce the warning locally, but could repro it in the kernel. [0] https://lore.kernel.org/lkml/20220330193352.GA119296@embeddedor/
| @@ -993,7 +993,7 @@ static void HUF_fillDTableX2Level2(HUF_DEltX2* DTable, U32 targetLog, const U32 | |||
|
|
|||
| static void HUF_fillDTableX2(HUF_DEltX2* DTable, const U32 targetLog, | |||
| const sortedSymbol_t* sortedList, | |||
| const U32* rankStart, rankVal_t rankValOrigin, const U32 maxWeight, | |||
| const U32* rankStart, rankValCol_t* rankValOrigin, const U32 maxWeight, | |||
There was a problem hiding this comment.
I fail to understand the relation between the -Wstringop-overflow warning, and this code change.
There is no string here, since rankValCol_t is a fixed-size array of U32 ?
There was a problem hiding this comment.
Yeah, it also does fixed size array overflow.
As far as I can tell, the code is correct as-is, but this works around a spurious warning.
There was a problem hiding this comment.
The exact warning is shown in the linked thread
There was a problem hiding this comment.
Reading the error message, the proposed solution seems to decay the fixed-size 2D array into a pointer of 1D arrays, thus negating the ability of the compiler to control array sizes. While this works, it feels like circumventing a control which should have passed properly.
The compiler seems incorrect in believing that the accessed memory region is only 52 bytes large, corresponding to a single 1D line of the 2D region.
The accessed memory region is defined as rankValCol_t rankVal[HUF_TABLELOG_MAX];, so it should be clear that it's actually an array or arrays, of total size is 624 bytes long.
The compiler fails to see that, and only considers the first line as being accessed.
A "proper solution" should be to report the issue to the compiler team and get it fixed, but given the timeline of such an operation, it doesn't solve the problem of passing compilation "now" without triggering the new warning.
Reading the symptom, I suspect a better fix would be to change the definition of rankVal into rankVal_t rankVal. It means the same thing, since rankVal_t is just a typedef for rankValCol_t[HUF_TABLELOG_MAX], but maybe it will help the compiler to realize that the accessed memory region is a single 2D array, which is large enough, as opposed to multiple consecutive 1D arrays.
I haven't reproduced the issue locally, even when using gcc-11, as in the report. This would be better to control that the proposed alternative fix works, but apparently, in user mode, this issue doesn't happen ?
There was a problem hiding this comment.
I tried that as well, but it didn’t work unfortunately.
I haven’t been able to repro it upstream, but was able to in the kernel.
I think it is just a false positive, and I don’t really care enough to try to find a more elegant fix. We can definitely report the issue though.
Backported from kernel patch [0].
I wasn't able to reproduce the warning locally, but could repro it in the kernel.
[0] https://lore.kernel.org/lkml/20220330193352.GA119296@embeddedor/