← Back to team overview

maria-developers team mailing list archive

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:

        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
        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?


Follow ups