This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: proposed patch for gcse.c (delete null pointer checks)



> AmigaOS has relevant information at address 4.

The concern is that delete-null-pointer-checks might
introduce bugs due to access of explicit low memory addresses.
Although I doubt this is a significant risk on the Amiga,
because I suspect such accesses happen in only a few routines
(e.g. run-time startup), this concern cannot be ignored
when it comes to operating system code.

Bugs due to "address 4" are elusive, so I will start
with a simpler bug in the current delete-null-pointer-checks: 

  #include <stdio.h>
  
  int *foop = 0;
  
  int main()
  {
    int *p = (int *) 0;
  
    printf("p is %d\n", *p);
    if (p)
      printf ("p is nonzero\n");
    else
      printf ("p is zero\n");
  
    return 0;
  }
  
On AIX (and no doubt Amiga) the above program
prints "p is nonzero" with "gcc -O3",
and prints "p is zero" with "gcc -O3  -fno-delete-null-pointer-checks".
Ironically, gcc deletes the "if" in both cases!
The value of p is zero, of course, and so gcc constant
propagation results in deletion the "if".
But unfortunately delete_null_pointer_checks() runs first
and produces the wrong answer.
(AIX cc gives the correct answer regardless of optimization level.)

I think a good fix for this bug is described in a comment
at the beginning of delete_null_pointer_checks() in gcse.c:
  /*  ... This could probably be integrated with global cprop
   with a little work.  */
The normal cprop deductions should take priority over
the possibly-incorrect non-null deduction,

Okay, back to "address 4".  The proposed patch to
delete_null_pointer_checks() can fail on address 4 if gcc:
   (a) transforms the reference to be a 4-byte offset
       from a register p known to contain zero.
   (b) does the dereference
   (c) tests the register to see if it contains a zero.
This (theoretical) bug would also be fixed by
ensuring that normal cprop takes priority.

============

But what if the value of the pointer is unknown?
E.g. suppose the above code had "int *p = foop;" instead,
which sets p to zero in a way that defeats normal cprop.
Then the program will erroneously print "p is nonzero"
even with the cprop/delete_null_pointer_checks integration.

A safe course would be NO-delete_null_pointer_checks
whenever low memory accesses are permitted.
But I consider that to be unnecessarily conservative.
Compilers routinely break code that "used to work" with optimizations
such as hoisting global variables out of loops, 
trashing variables across setjmp/longjmp, etc.
(I could mention ANSI aliasing, but that would be a cheap shot :-)
In contrast, I do not think we have yet seen a real bug 
caused by delete_null_pointer_checks,
not even one due to an implementation bug.
I recommend, and urge, that we give this optimization a chance,
and wait for evidence of actual problems
before dumbing it down or turning it off.

Tom Truscott


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]