Bug 30785

Summary: Test to null pointer optimised away at -O2
Product: gcc Reporter: etienne_lorrain
Component: cAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED INVALID    
Severity: normal CC: gcc-bugs
Priority: P3    
Version: 4.1.1   
Target Milestone: 4.1.2   
Host: i686-pc-cygwin Target: powerpc-eabi
Build: i686-pc-cygwin Known to work: 4.1.2 4.0.3
Known to fail: 4.1.1 4.1.0 Last reconfirmed:

Description etienne_lorrain 2007-02-13 11:01:18 UTC
$ cat tmp.c
typedef unsigned size_t;
char *xxcpy(  char *pDest, const char *pSrc, size_t n);
char *strncpy(  char *pDest, const char *pSrc, size_t n) {
  if (pDest == 0 || pSrc == 0) {
      if (pDest)
          *pDest = 0;
      return pDest;
      }
  return xxcpy (pDest, pSrc, n);
  }
$ powerpc-eabi-gcc -Wall -S tmp.c -o tmp.s -O2 -mcpu=440
$ cat tmp.s .file   "tmp.c"
        .section        ".text"
        .align 2
        .globl strncpy
        .type   strncpy, @function strncpy: stwu 1,-8(1)
        mflr 0
        stw 0,12(1)
        bl xxcpy
        lwz 0,12(1)
        addi 1,1,8
        mtlr 0
        blr
        .size   strncpy, .-strncpy
        .ident  "GCC: (GNU) 4.1.1"
$ powerpc-eabi-gcc -Wall -S tmp.c -o tmp.s -O1 -mcpu=440
$ cat tmp.s .file   "tmp.c"
        .section        ".text"
        .align 2
        .globl strncpy
        .type   strncpy, @function strncpy: stwu 1,-8(1)
        mflr 0
        stw 0,12(1)
        cmpwi 0,3,0
        beq- 0,.L2
        cmpwi 7,4,0
        bne+ 7,.L4
        stb 4,0(3)
        b .L2 .L4: bl xxcpy .L2: lwz 0,12(1)
        mtlr 0
        addi 1,1,8
        blr
        .size   strncpy, .-strncpy
        .ident  "GCC: (GNU) 4.1.1"
$ powerpc-eabi-gcc -v
Using built-in specs.
Target: powerpc-eabi Configured with: ../gcc-4.1.1/configure --target=powerpc-eabi --with-cpu=440 --enable-languages=c,c++
Thread model: single
gcc version 4.1.1 

  Seems quite clear some optimisation assumes pointers cannot be null.
  Is there a flag to disable this option?
  Thanks,
  Etienne.
Comment 1 Richard Biener 2007-02-13 13:00:07 UTC
I suppose this is done by the VRP pass (-f[no-]tree-vrp) so target independent.  If so, this is fixed on current 4.1 branch.  Can you verify this indeed is
broken by VRP by looking at and before the vrp tree dumps? (-fdump-tree-all)
Comment 2 etienne_lorrain 2007-02-13 13:44:28 UTC
 Problem is fixed by -fno-tree-vrp and -O2
 I am not a specialist of -fdump-tree-all, but it seems like that:
 tmp.c.t35.copyprop1:

;; Function strncpy (strncpy)

strncpy (pDest, pSrc, n)
{
  char * D.1282;
  char * D.1281;

<bb 0>:
  if (pDest_2 == 0B) goto <L1>; else goto <L0>;

<L0>:;
  if (pSrc_5 == 0B) goto <L1>; else goto <L4>;

<L1>:;
  if (pDest_2 != 0B) goto <L2>; else goto <L3>;

<L2>:;
  *pDest_2 = 0;

<L3>:;
  pDest_4 = pDest_2;
  goto <bb 6> (<L5>);

<L4>:;
  pDest_7 = xxcpy (pDest_2, pSrc_5, n_6);
  pDest_8 = pDest_7;

  # pDest_1 = PHI <pDest_2(4), pDest_7(5)>;
<L5>:;
  return pDest_1;

}
-------------------
  tmp.t36.vrp, the test disappear:

;; Function strncpy (strncpy)


SSA replacement table
N_i -> { O_1 ... O_j } means that N_i replaces O_1, ..., O_j

pDest_12 -> { pDest_2 }
pDest_13 -> { pDest_2 }
pDest_14 -> { pDest_2 }
pDest_15 -> { pDest_2 }
pSrc_16 -> { pSrc_5 }

Number of virtual NEW -> OLD mappings:       0
Number of real NEW -> OLD mappings:          5
Number of total NEW -> OLD mappings:         5

Number of virtual symbols: 0


Incremental SSA update started at block: 0

Number of blocks in CFG: 9
Number of blocks to update: 9 (100%)




Value ranges after VRP:

pDest_1: VARYING
pDest_2: ~[0B, 0B]  EQUIVALENCES: { } (0 elements)
<retval>_3: VARYING
pSrc_5: ~[0B, 0B]  EQUIVALENCES: { } (0 elements)
pDest_7: VARYING
pDest_8: [pDest_7, pDest_7]  EQUIVALENCES: { pDest_7 } (1 elements)
pDest_15: ~[0B, 0B]  EQUIVALENCES: { pDest_2 } (1 elements)
pSrc_16: ~[0B, 0B]  EQUIVALENCES: { pSrc_5 } (1 elements)


Folding predicate pDest_2 == 0B to 0
Folding predicate pSrc_5 == 0B to 0
Folding predicate pDest_2 != 0B to 1
Removing basic block 7
Removing basic block 2
Removing basic block 8
Removing basic block 3
Removing basic block 4
Removing basic block 1
Merging blocks 0 and 5
Merging blocks 0 and 6
strncpy (pDest, pSrc, n)
{
  char * D.1282;
  char * D.1281;

<bb 0>:
  pDest_7 = xxcpy (pDest_2, pSrc_5, n_6);
  pDest_8 = pDest_7;
  return pDest_7;

}

  Thanks,
  Etienne.
Comment 3 Richard Biener 2007-02-13 13:57:40 UTC
This is indeed fixed on the branch.
Comment 4 Thorsten Glaser 2007-03-10 20:31:50 UTC
It's not fixed on 4.1 branch:

tglaser@hephaistos:~/tmp $ cat _t.c
typedef unsigned size_t;
char *xxcpy(  char *pDest, const char *pSrc, size_t n);
char *strncpy(  char *pDest, const char *pSrc, size_t n) {
  if (pDest == 0 || pSrc == 0) {
      if (pDest)
          *pDest = 0;
      return pDest;
      }
  return xxcpy (pDest, pSrc, n);
  }

Neither with 4.1.2 (release):

tglaser@hephaistos:~/tmp $ ~/tmp/gp/bin/gcc -O2 -S _t.c -o -
        .file   "_t.c"
        .text
        .p2align 4,,15
.globl strncpy
        .type   strncpy, @function
strncpy:
        pushl   %ebp
        movl    %esp, %ebp
        popl    %ebp
        jmp     xxcpy
        .size   strncpy, .-strncpy
        .ident  "GCC: (GNU) 4.1.2"
        .section        .note.GNU-stack,"",@progbits

Nor with today's weekly snapshot:

tglaser@hephaistos:~/tmp $ ~/tmp/gp/bin/gcc -O2 -S _t.c -o -
        .file   "_t.c"
        .text
        .p2align 4,,15
.globl strncpy
        .type   strncpy, @function
strncpy:
        pushl   %ebp
        movl    %esp, %ebp
        popl    %ebp
        jmp     xxcpy
        .size   strncpy, .-strncpy
        .ident  "GCC: (GNU) 4.1.3 20070305 (prerelease)"
        .section        .note.GNU-stack,"",@progbits

For the meantime, I'm using -fno-tree-vrp as well, but this
is a serious bug, please fix it.

(tried to reopen the PR but that didn't work)
Comment 5 Richard Biener 2007-03-10 20:55:36 UTC
Reopening.  It's fixed for 64bit systems but not 32bit ones!?  vrp dump difference:

 Visiting statement:
-D.1617_3 = pDest_2 == 0B;
+D.1286_3 = pDest_2 == 0B;

-Found new range for D.1617_3: VARYING
+Found new range for D.1286_3: [0, 0]


 Visiting statement:
-D.1618_5 = pSrc_4 == 0B;
+D.1287_5 = pSrc_4 == 0B;

-Found new range for D.1618_5: VARYING
+Found new range for D.1287_5: [0, 0]


 Visiting statement:
-D.1619_6 = D.1617_3 || D.1618_5;
+D.1288_6 = D.1286_3 || D.1287_5;

-Found new range for D.1619_6: VARYING
+Found new range for D.1288_6: [0, 0]


and now bad things happen...
Comment 6 Richard Biener 2007-03-10 21:24:34 UTC
Actually, strncpy is a reserved name so we assume pDest and pSrc are non-null.
Comment 7 Richard Biener 2007-03-10 21:29:48 UTC
You can use -fno-builtin-strncpy or -ffreestanding to "fix" this.