[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)


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;

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

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
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. :)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]