[Bug target/21479] New: optimizer removes incorrectly variable comparision in if clause

[Bug target/21479] New: optimizer removes incorrectly variable comparision in if clause

[chaac at nic dot fi]

Hi,

avr-gcc seems to occasinally remove compare for ppv in following code. This 
changes semantics in the if clause and causes incorrect code execution as ppv is 
not compared first before *ppv compare.

void test(int req, void *conf)
{
    void **ppv = (void **) conf;
    unsigned long *lvp = (unsigned long *) conf;
    unsigned long lv = *lvp;
    unsigned char bv = (unsigned char) lv;

    switch (req) {
    case 1:
        if (bv)
        {
            asm ( "nop ; bv needs to be defined and used");
        }
        break;

    case 2:
        asm ( "nop ; entry to case 2");

        /* bug: first compare for ppv is missing, second is only left */
        if (ppv && (*ppv != 0)) {
            asm ( "nop ; ppv and *ppv are not zeros");
        } else {
            asm ( "nop ; either ppv or *ppv is zero");
        }

        break;
    }
}

Known workaround is to use -fno-delete-null-pointer-checks.

After a small debate on this issue on avr-gcc mailing list I was adviced to post 
a bug report.

I used following command line:
avr-gcc -c -mmcu=atmega128 -Os -Wall -Wstrict-prototypes -Wa,-ahlms=test.lst 
test.c -o test.o 

I am using avr-gcc supplied in WinAVR-20050214 (gcc 3.4.3).

> avr-gcc -v
Reading specs from C:/WinAVR/bin/../lib/gcc/avr/3.4.3/specs
Configured with: ../gcc-3.4.3/configure --prefix=m:/WinAVR --build=mingw32 --
host=mingw32 --target=avr --enable-languages=c,c++ --with-dwarf2
Thread model: single
gcc version 3.4.3

-- 
           Summary: optimizer removes incorrectly variable comparision in if
                    clause
           Product: gcc
           Version: 3.4.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: chaac at nic dot fi
                CC: gcc-bugs at gcc dot gnu dot org
  GCC host triplet: i686-winxp-sp2
GCC target triplet: avr-*-*


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-05-09 22:28 -------
I don't think this is a bug since conf and ppv cannot be null as you deferenced them already and would 
trap on most machines.  (there is another bug about this recently filed too).

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-05-09 22:31 -------
Do you have a pointer to the mail on that list?

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-05-09 22:32 -------
Oh, one more thing, deferencing a null pointer is undefined by the C standard.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |21305


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[schlie at comcast dot net]

------- Additional Comments From schlie at comcast dot net  2005-05-09 23:19 -------
(In reply to comment #1)
> I don't think this is a bug since conf and ppv cannot be null as you deferenced them already
> and would  trap on most machines.  (there is another bug about this recently filed too).
> Oh, one more thing, deferencing a null pointer is undefined by the C standard.

??? Although dereferencing a null pointer may be "undefined" as some machines MAY trap,
 it certainly doesn't give a conformant C compiler license to ignore a comparison of a
pointer against null, which is well defined.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[marcus at jet dot franken dot de]

------- Additional Comments From marcus at jet dot franken dot de  2005-05-10 06:31 -------
see comment #1 ...  
  
you already derefenced the pointer in ppv (in the line  
 unsigned long lv = *lvp; 
) 
 
so the compiler assumes that anohter NULL ptr check is not needed. 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[chaac at nic dot fi]

------- Additional Comments From chaac at nic dot fi  2005-05-10 08:00 -------
Andrew,

Here is a pointer to the mailing list:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

Topic started on May 08, 2005 with subject "WinAVR 20050214 (gcc 3.4.3) and 
optimizer bug."


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[chaac at nic dot fi]

------- Additional Comments From chaac at nic dot fi  2005-05-10 08:06 -------
In AVR's reading memory address 0 is valid thing though. In a way I can 
understand why to optimization in x86 but shouldn't this be disabled by default 
on AVR's?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[schlie at comcast dot net]

------- Additional Comments From schlie at comcast dot net  2005-05-10 08:31 -------
(In reply to comment #5)
> see comment #1 ...  
>   
> you already derefenced the pointer in ppv (in the line  
>  unsigned long lv = *lvp; 
> ) 
>  
> so the compiler assumes that anohter NULL ptr check is not needed. 

- yes, however as the loigical extention of:
   "a null reference is undefined" => "may trap" => "will trap"
   is simply wrong, and is not justifyable; such an optimization
   is target specific, as it depends on "will trap" target semantics.

   (not to mention that even if it is trapped for a particular target,
    that the target won't simply return some value, so pointer null
    comparsions can't be reliably optimized away unless the compiler
    can also enforce dereferenced null pointer trap semantics for that
    particualr target, which GCC does not appear to do.)



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[chaac at nic dot fi]

------- Additional Comments From chaac at nic dot fi  2005-05-10 08:41 -------
Sorry about wrong mailing list pointer :)

http://lists.gnu.org/archive/html/avr-gcc-list/2005-05/index.html

Here is correct URL :)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[ericw at evcohs dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ericw at evcohs dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[falk at debian dot org]

------- Additional Comments From falk at debian dot org  2005-05-17 12:30 -------
(In reply to comment #8)
> - yes, however as the loigical extention of:
>    "a null reference is undefined" => "may trap" => "will trap"
>    is simply wrong, and is not justifyable; such an optimization
>    is target specific, as it depends on "will trap" target semantics.

Right. However, the logic here is simply "a null pointer dereference is
undefined" => "if you still do it, your code may behave however gcc feels
like", which is backed by the C standard. So this is invalid.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479

[Bug target/21479] optimizer removes incorrectly variable comparision in if clause

[schlie at comcast dot net]

------- Additional Comments From schlie at comcast dot net  2005-05-17 21:24 -------
(In reply to comment #10)
> (In reply to comment #8)
> > - yes, however as the loigical extention of:
> >    "a null reference is undefined" => "may trap" => "will trap"
> >    is simply wrong, and is not justifyable; such an optimization
> >    is target specific, as it depends on "will trap" target semantics.
> 
> Right. However, the logic here is simply "a null pointer dereference is
> undefined" => "if you still do it, your code may behave however gcc feels
> like", which is backed by the C standard. So this is invalid.

No, only the "null pointer dereference" itself is undefined. which means
that upon a null pointer reference any or no value may be returned.

Is says, implies, and grants no rights what so ever to an implementation,
to define that an arbitrary behavior will occure which may be subsequenlty
relied upon to occured unless the implementation inforces that behavior.

More specifically, unless GCC can warrent that a "null pointer dereference"
will trap will terminate program execution, it must preserve the semantics
of the remaining programs execution as defined by the standard, which
includes but not limited to preserving null-pointer comparision semantics,
as defined by the standard; as not to do so would be in violation of the same.





-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21479