maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #07381
Re: WebScaleSQL: Actually fix all the array bounds warnings
Hi, Sergey!
On May 28, Sergey Vojtovich wrote:
> Hi Sergei and Alexey,
>
> during our recent discussion re WebScaleSQL patches we agreed to review $subj:
> https://github.com/webscalesql/webscalesql-5.6/commit/094187c4a20e8d0c02f17210da5c80bc0db18f17
>
> All these warnings come from strings/decimal.c. I was able to reproduce them using
> gcc-4.8.1, cmake -DBUILD_CONFIG=mysql_release (basically needs -Wall -O3).
>
> I simplified this function to the following:
> gcc -Wall -O3 -c dec.c -o dec.o
>
> static const int powers10[2 + 1]= { 1, 10, 100 };
>
> int remove_leading_zeroes(int decimals, int var)
> {
> decimals%= 2;
> while (var < powers10[decimals--]) /* no-op */;
> return decimals;
> }
>
> The only allowed value for decimals is 1 and var must be > 0. Similar assertion
> is enforced by original function.
This is interesting. In fact, the compiler cannot know that var must be
greater than zero, so I've made variables unsigned and added 0 to
powers10 array:
static const int powers10[]= { 0, 1, 10, 100 };
int remove_leading_zeroes(unsigned int decimals, unsigned int var)
{
decimals%= 2;
while (var < powers10[decimals--]) ;
return decimals;
}
This looks perfectly safe to me. But there is still a warning. Now,
let's see the assembly:
remove_leading_zeroes:
andl $1, %edi
movl %edi, %edx
leal -1(%rdi), %eax
cmpl powers10(,%rdx,4), %esi
jae .L2
movl %eax, %ecx
leal -2(%rdi), %edx
cmpl powers10(,%rcx,4), %esi
movl %edx, %eax
jae .L2
leal -3(%rdi), %eax
subl $4, %edi
cmpl powers10(,%rdx,4), %esi
cmovb %edi, %eax
.L2:
rep ret
This roughly corresponds to
decimals&= 1;
if (var >= powers10[decimals ]) return decimals - 1;
if (var >= powers10[decimals-1]) return decimals - 2;
if (var >= powers10[decimals-2]) return decimals - 3;
return decimals-4;
The number of if's is equal to the number of elements in the array
(adding more elements I've got more if's). Which, really, makes no
sense, the size of the array is not used as a loop condition.
I've no idea why gcc is doing that. A compiler bug?
Regards,
Sergei
Follow ups
References