Re: gcc-2.95 OK, gcc-{3,4}.X not OK

Re: gcc-2.95 OK, gcc-{3,4}.X not OK

[Andris Kalnozols]

> [redir to gcc-help]
>
> On 01/01/2010 05:44 AM, Andris Kalnozols wrote:
> 
> > If the bug was a basic programming error, one would expect a
> > runtime failure (dereferencing a NULL pointer) no matter which
> > compiler was used.
> 
> I would not expect that, and I have no idea whay you would.  Undefined
> behaviour can happen in any way: maybe the program appears to run
> correctly, maybe it faults.

I stand corrected.  Dumb luck was hiding a programming flaw.

> > The application compiles cleanly with no warnings using "-Wall".
> > Were there any transition issues with the newer gcc compilers of
> > which I may not be aware?
> 
> No.  As you've done the obvious first stage (using -Wall) you now
> should run your program under Valgrind, assuming that it is available
> on your system.

Having already looked at Valgrind a few years ago, I revisited it per
your suggestion and studied its options this time.  With the help of
this website:

  https://computing.llnl.gov/code/memcheck/#deciphering4

the Valgrind application revealed this flaw:

  pcptr = pcptr->code = nop;

"code" can't be reached through "pcptr" if the pointer does not
yet have the proper address.

Since multiple assignments are legal and evaluated from right to left,
one could expect the following to work:

  pcptr->code = pcptr = nop;

It does *not* work, however, using the gcc 3.3 or 4.3 compilers.
To summarize:

  pcptr = pcptr->code = nop;   /* crashes with no compiler warning */
  pcptr->code = pcptr = nop;   /* crashes with the warning:
                                * operation on `pcptr' may be undefined
                                */
  pcptr->code = (pcptr = nop); /* same as above */
  pcptr = nop;  pcptr->code = nop;  /* this works */

FWIW, the HP-UX 11.31 compiler warns/not warns in the same
way but the compiled program is "luckier" at run time.

Perhaps the compiler should regard the above bootstrapping-style
multiple assignments as ambiguous and issue the warning regardless
of the order in which the targets appear.

Valgrind is an awesome tool.

------
Andris

Re: gcc-2.95 OK, gcc-{3,4}.X not OK

[Andrew Haley]

On 01/03/2010 04:54 AM, Andris Kalnozols wrote:

> Since multiple assignments are legal and evaluated from right to left,
> one could expect the following to work:
> 
>   pcptr->code = pcptr = nop;

I wouldn't.  :-)

> It does *not* work, however, using the gcc 3.3 or 4.3 compilers.
> To summarize:
> 
>   pcptr = pcptr->code = nop;   /* crashes with no compiler warning */
>   pcptr->code = pcptr = nop;   /* crashes with the warning:
>                                 * operation on `pcptr' may be undefined
>                                 */
>   pcptr->code = (pcptr = nop); /* same as above */
>   pcptr = nop;  pcptr->code = nop;  /* this works */
> 
> FWIW, the HP-UX 11.31 compiler warns/not warns in the same
> way but the compiled program is "luckier" at run time.
> 
> Perhaps the compiler should regard the above bootstrapping-style
> multiple assignments as ambiguous and issue the warning regardless
> of the order in which the targets appear.

Well, gcc doesn't necessarily know.  We do a fair bit of analysis in
an attempt to help the programmer, and we warn when we reasonably can,
but gcc isn't omniscient.

In this case, though, you were burnt by a pointlessly obscure coding
style, combined with a misunderstanding of the language.  If you write
the above code as

   pcptr = nop;
   pcptr->code = nop;

it'll always work, and its semantics are obvious.

Andrew.

Re: gcc-2.95 OK, gcc-{3,4}.X not OK

[Andris Kalnozols]

> > On 01/03/2010 04:54 AM, Andris Kalnozols wrote:
> 
> > Since multiple assignments are legal and evaluated from right
> > to left, one could expect the following to work:
> > 
> >   pcptr->code = pcptr = nop;
> 
> On 01/03/2010 11:17 AM, Andrew Haley replied:
>
> I wouldn't.  :-)

OK, the problem was found, not by Valgrind nor Coverity, but the
old fashioned method of inserting debugging statements in the code.

There are no coding nor compilation errors with either

  pcptr = pcptr->code = nop;
    or
  pcptr->code = pcptr = nop;

As long as the pointers are actually pointing to something,
this is typical linked list processing.

There is a problem, however, when a global pointer structure is
the assignment target of a function which itself may modify the
global pointer.  The included test program gives the following
output when compiled with gcc {3,4}.X:


  ./gcc-bug
  Take the suspect branch [1] or stay `safe' [2]? 1
  Initialize `pcptr' with NULL [1] or malloc [2]? 1

  main:    [init] pcptr       = (nil)

  Taking branch 1.
  fnc_A:          pcptr       = 0x8fff008
                  pcptr->code = (nil)
  Segmentation fault


  ./gcc-bug
  Take the suspect branch [1] or stay `safe' [2]? 1
  Initialize `pcptr' with NULL [1] or malloc [2]? 2

  main:    [init] pcptr       = 0x8c39008

  Taking branch 1.
  fnc_A:          pcptr       = 0x8c39018
                  pcptr->code = (nil)
  main:           pcptr       = 0x8c39018 (0x8c39018 <- should be this)
                  pcptr->code = (nil)     (0x8c39018 <- should be this)


Needless to say, the nature and duration of this behavior
through the 3.X and current versions of gcc certainly seems
to go against the Principle of Least Astonishment.

------
Andris


......................................................................


#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

#define PC              struct pc
typedef unsigned short  word;

PC {                    /* typical linked list structure */
    word   op;
    PC    *code;
};

PC *pcptr;              /* global structures */
PC *pcptr_copy;

int get_data(char *prompt)
{
    char buf[40];
    int result, *result_ptr;

    result_ptr = &result;
    do {
        fputs(prompt, stdout);
        fgets(buf, sizeof(buf), stdin);
        sscanf(buf, "%i", result_ptr);
    } while (result != 1 && result != 2);
    return(result);
}

PC *fnc_A(void)
{
    if ((pcptr = pcptr_copy = (PC *)malloc(sizeof(PC))) == NULL) {
        perror("malloc(3) failed");
        exit(1);
    }
    pcptr->code = NULL;
    printf("fnc_A:  \tpcptr       = %p\n", (void *)pcptr);
    printf("        \tpcptr->code = %p\n", (void *)pcptr->code);
    return(pcptr);
}

int main(int argc, char *argv[])
{
    int init_choice, branch;

    branch      = get_data("Take the suspect branch [1] or stay `safe' [2]? ");
    init_choice = get_data("Initialize `pcptr' with NULL [1] or malloc [2]? ");
    puts("");

    if (init_choice == 1)
        pcptr = NULL;
    else {
        if ((pcptr = (PC *)malloc(sizeof(PC))) == NULL) {
            perror("malloc(3) failed");
            exit(1);
        }
    }
    printf("main:    [init] pcptr       = %p\n\n", (void *)pcptr);

    printf("Taking branch %i.\n", branch);
    if (branch == 1)
        /*
         * This branch works fine with the gcc-2.95 and HP-UX compilers.
         * If compiled with gcc {3,4}.X, however, the results are:
         *
         *   > SEGFAULT if `pcptr' is initialized to NULL
         *       or
         *   > Corrupted value of `pcptr->code' if `pcptr'
         *     is initialized to a non-NULL address.
         *
         * The newer compilers appear to incorrectly handle the case
         * where the assignment target is a global pointer reference
         * (pcptr->code) and the value being assigned is returned by
         * a function which can modify the global pointer itself.
         */
        pcptr->code = fnc_A();
    else {
        /*
         * All compilers can handle this branch correctly
         * regardless of how `pcptr' is initialized.
         */
        PC *tmp_ptr;
        tmp_ptr     = fnc_A();
        pcptr->code = tmp_ptr;
    }
    printf("main:   \tpcptr       = %p", (void *)pcptr);
    printf("\t(%p <- should be this)\n", (void *)pcptr_copy);
    printf("        \tpcptr->code = %p", (void *)pcptr->code);
    printf("\t(%p <- should be this)\n", (void *)pcptr_copy);
    return(0);
}

Re: gcc-2.95 OK, gcc-{3,4}.X not OK

[Cedric Roux]

Andris Kalnozols wrote:
> There are no coding nor compilation errors with either
> 
>   pcptr = pcptr->code = nop;
>     or
>   pcptr->code = pcptr = nop;
> 
> As long as the pointers are actually pointing to something,
> this is typical linked list processing.

Well, this is debatable.
If by:
   pcptr->code = pcptr = nop;
you mean:
   pcptr = nop;
   pcptr->code = nop;
(assuming nop == NULL)
then in the second assignment pcptr is NULL,
you dereference a NULL pointer. (And you're
lucky you get a segmentation fault, on some
architectures you have no crash at all and
left with a funky bug.)

>         pcptr->code = fnc_A();

What does that mean in your head?
fnc_A modifies pcptr, so you expect
pcptr in pcptr->code to be the value
before the call or after?

In a hypothetical assembly language, your statement could be:

   load  (pcptr), reg1
   add   #offset(code in PC), reg1
   call  fnc_A
   store regret, (reg1)

Or do you expect:

   call  fnc_A
   load  (pcptr), reg1
   add   #offset(code in PC), reg1
   store regret, (reg1)

This is quite different. I don't know what gcc does, but
I know your pattern is weird and I guess you
should avoid it in production code. It will
give some headaches to those who will need
to maintain the code.

Cédric.

Re: gcc-2.95 OK, gcc-{3,4}.X not OK

[Andrew Haley]

On 01/07/2010 10:37 PM, Andris Kalnozols wrote:
>>> On 01/03/2010 04:54 AM, Andris Kalnozols wrote:
>>
>>> Since multiple assignments are legal and evaluated from right
>>> to left, one could expect the following to work:
>>>
>>>   pcptr->code = pcptr = nop;
>>
>> On 01/03/2010 11:17 AM, Andrew Haley replied:
>>
>> I wouldn't.  :-)
> 
> OK, the problem was found, not by Valgrind nor Coverity, but the
> old fashioned method of inserting debugging statements in the code.
> 
> There are no coding nor compilation errors with either
> 
>   pcptr = pcptr->code = nop;
>     or
>   pcptr->code = pcptr = nop;
> 
> As long as the pointers are actually pointing to something,
> this is typical linked list processing.
> 
> There is a problem, however, when a global pointer structure is
> the assignment target of a function which itself may modify the
> global pointer.  The included test program gives the following
> output when compiled with gcc {3,4}.X:
> 
> 
>   ./gcc-bug
>   Take the suspect branch [1] or stay `safe' [2]? 1
>   Initialize `pcptr' with NULL [1] or malloc [2]? 1
> 
>   main:    [init] pcptr       = (nil)
> 
>   Taking branch 1.
>   fnc_A:          pcptr       = 0x8fff008
>                   pcptr->code = (nil)
>   Segmentation fault
> 
> 
>   ./gcc-bug
>   Take the suspect branch [1] or stay `safe' [2]? 1
>   Initialize `pcptr' with NULL [1] or malloc [2]? 2
> 
>   main:    [init] pcptr       = 0x8c39008
> 
>   Taking branch 1.
>   fnc_A:          pcptr       = 0x8c39018
>                   pcptr->code = (nil)
>   main:           pcptr       = 0x8c39018 (0x8c39018 <- should be this)
>                   pcptr->code = (nil)     (0x8c39018 <- should be this)
> 
> 
> Needless to say, the nature and duration of this behavior
> through the 3.X and current versions of gcc certainly seems
> to go against the Principle of Least Astonishment.

I'm sorry, this comment is too opaque for me to understand.  gcc
doesn't seem to be doing anything wrong.  The order of evaluation of
subexpressions is unspecified, sure, but that's how C has always
worked.  What is astonishing about it?

Andrew.