← Back to team overview

maria-developers team mailing list archive

Re: WebScaleSQL: Actually fix all the array bounds warnings

 

Hi Sergei,

I reported gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61520

Regards,
Sergey

On Fri, Jun 13, 2014 at 05:55:43PM +0200, Sergei Golubchik wrote:
> 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
> 


References