[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Bug-ed] Aliasing violations in ed cause segfaults.
From: |
Harald van Dijk |
Subject: |
[Bug-ed] Aliasing violations in ed cause segfaults. |
Date: |
Wed, 30 Apr 2008 11:04:03 +0200 |
User-agent: |
Mutt/1.5.16 (2007-06-09) |
Hi,
Aliasing violations are a fun thing. They are found by the
compiler, the compiler warns the user, the user does not see a
direct problem, and modifies the code so that the compiler doesn't
warn about it anymore. Now comes a newer compiler, with newer
optimisations, and the incorrect code does cause problems, but the
warning about it is still suppressed. *Please* don't do that unless
the code is actually correct.
In this case, the first problem is in carg_parser.[ch]:
typedef struct
{
ap_Record * data;
char * error;
int data_size;
int error_size;
}
Arg_parser;
char ap_resize_buffer( char **buf, int *size, const int min_size );
char push_back_record( Arg_parser * ap, const int code, const char * argument )
{
/* ... */
if( !ap_resize_buffer( (char **)(void *)&(ap->data), 0,
( ap->data_size + 1 ) * sizeof( ap_Record ) ) )
return 0;
/* ... */
}
ap->data is a pointer to an ap_Record. If the cast to void * is
removed, GCC can warn about it:
carg_parser.c: In function ‘push_back_record’:
carg_parser.c:44: warning: dereferencing type-punned pointer will break
strict-aliasing rules
With the cast, GCC is told to shut up about it, but still optimises
on the assumption that ap->data is never actually accessed as a
char *.
Now, for most systems, that doesn't matter. The optimiser is not
smart enough to actually do anything with that knowledge. But a
user found a combination of compiler and compiler flags that change
that. When ed is compiled with GCC 4.2 (4.2.2 or 4.2.3, at least), and
the compiler flags include
-march=pentium-m
-O2
-fomit-frame-pointer
-finline-functions
-fpeel-loops
or these options are enabled for other reasons (such as -O3 or
-fprofile-use), ed fails at parsing command-line options. Horribly.
In fact, if an attempt is made to run the testsuite, literally
every single test fails.
It's not just carg_parser, either. Although carg_parser is the
reason ed bombs here, as pointed out by gdb:
Program received signal SIGSEGV, Segmentation fault.
push_back_record (ap=0xffd644a8, code=0, argument=0xffd65176 "/dev/null") at
carg_parser.c:48
48 p->code = code;
(gdb) bt
#0 push_back_record (ap=0xffd644a8, code=0, argument=0xffd65176 "/dev/null")
at carg_parser.c:48
#1 0x0804cb83 in ap_init (ap=0xffd644a8, argc=2, argv=0xffd64564,
options=0x80531e0, in_order=0 '\0') at carg_parser.c:244
#2 0x0804dee7 in main (argc=Cannot access memory at address 0x0
) at main.c:150
(gdb) p p
$1 = (ap_Record *) 0x0
...similar aliasing violations exist in other parts of the program as
well.
A fix requires either defining every pointer that's accessed as a
char * as such, or not accessing those pointers as a char *. The
former is easy but horribly ugly. It's so ugly that I initially
decided to not mail you about the problem just yet. If you want to
see it anyway, I've attached it to the Gentoo bugreport, and it's
available at
http://bugs.gentoo.org/attachment.cgi?id=142426&action=view
The latter, not accessing those pointers as a char * would probably
be a better solution. However, taking that approach requires
reworking the resize_buffer logic. For ap_resize_buffer, that's not
so much of a problem (return a void *, let the caller convert it to
the proper type, like realloc), but for resize_buffer, it is, because
of the disable_ and enable_interrupts calls. I haven't been able to
fix it myself in that approach in a clean way, but I hope you have
better luck.
(Of course, there is a third possibility: unconditionally force
-fno-strict-aliasing in the compiler options. This would work, but
it may break compilation with other compilers if they make the same
optimisation as GCC, but have different command-line options, or no
command-line options, to disable it.)
If there's anything I can do to help, please let me know. And
thanks for looking after ed. I'm glad someone does, because I like
it. :)
- [Bug-ed] Aliasing violations in ed cause segfaults.,
Harald van Dijk <=