Bug 43774 - option -O2 generates wrong assembly code
Summary: option -O2 generates wrong assembly code
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.4.0
: P3 critical
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-17 03:33 UTC by dirtysalt
Modified: 2010-04-19 02:25 UTC (History)
2 users (show)

See Also:
Host: GNU/linux 2.6.9 Intel Xeon
Target: GNU/linux 2.6.9 Intel Xeon
Build: GNU/linux 2.6.9 Intel Xeon
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dirtysalt 2010-04-17 03:33:19 UTC
compiling the following code with -O2, the program will core dump.
I check the assembly code output, it seems the 'strlen' function call is replaced by the 'builtin strlen' funciton and will read the first four byte on a invalid memory page.
And if i replace the mmap with malloc and run under the Valgrind[3.5.0],the Valgrind also reports 'Invalid read of size 4'.

Ps:How to workaround this piece of code???I think there are two ways
a.mmap 4 bytes or more to make sure the strlen will not read the invalid memory
b.use the gcc option '-fno-builtin-strlen' to make sure the 'strlen' is not replaced.
But I'm not sure is there a more elegant way to workaroud this??

====================================================================
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/mman.h>
typedef struct _x_t
{
    int offset; //'strlen' is replaced iff. this field exists.
    char data[0];
}x_t;

int main()
{   
    //make a 4K memory page.
    char *buff=mmap(NULL,4096,PROT_WRITE | PROT_READ ,MAP_PRIVATE | MAP_ANONYMOUS,0,0);
    char *buffer = buff+4096-11;
    strcpy(buffer,"0123456789");  
    x_t *x=(x_t*)buffer;
    printf("%d\n",strlen(x->data)); //read a invalid page.
    munmap(buff,4096);
    return 0;
}
Comment 1 Jakub Jelinek 2010-04-17 06:55:10 UTC
You should make the struct packed, because otherwise you are accessing it unaligned.
Comment 2 dirtysalt 2010-04-17 08:11:53 UTC
(In reply to comment #1)
> You should make the struct packed, because otherwise you are accessing it
> unaligned.
>
 
I think the main problem coming from that GCC replace the 'strlen' function with the following code.

.L2:
        movl    (%ecx), %eax //$ecx is the beginning of string..
        addl    $4, %ecx //seems to access the content in 4 bytes each time.
        leal    -16843009(%eax), %edx
        notl    %eax
        andl    %eax, %edx
        andl    $-2139062144, %edx
        je      .L2

But it seems the piece of code can only be used when the valid memory size is 4*N,otherwise it will make a 'Invalid read'...

So under what sitaution, GCC will replace 'strlen' call with the above code. 
Comment 3 dirtysalt 2010-04-17 09:45:14 UTC
(In reply to comment #1)
> You should make the struct packed, because otherwise you are accessing it
> unaligned.
> 

If I recompile it with -O0, the assembly code call 'strlen' function.And I use Valgrind to run it,there is no 'Invalid read'
Comment 4 Andreas Schwab 2010-04-17 09:49:27 UTC
The compiler can assume that *x is correctly aligned, so this is not a bug.
Comment 5 dirtysalt 2010-04-17 10:28:11 UTC
(In reply to comment #1)
> You should make the struct packed, because otherwise you are accessing it
> unaligned.
> 

3ku for your reply.Your reply is much more useful than Axxx's reply.3ku very much:).
Comment 6 Andrew Pinski 2010-04-19 01:08:59 UTC
Just a littke more: alignof (struct _x_t) is 4 therefor so is the alignof(_x_t
::data).
Comment 7 dirtysalt 2010-04-19 02:25:10 UTC
(In reply to comment #6)
> Just a littke more: alignof (struct _x_t) is 4 therefor so is the alignof(_x_t
> ::data).
> 
3ku very much,with your explanation I think I can understand it..3ku very much.