Bug 22051 - [4.0/4.1 regression] Wrong code for function pointer comparision during optimization
[4.0/4.1 regression] Wrong code for function pointer comparision during optim...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: tree-optimization
4.0.0
: P2 normal
: 4.0.1
Assigned To: Not yet assigned to anyone
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-06-13 16:14 UTC by Randolph Chung
Modified: 2005-07-01 19:04 UTC (History)
7 users (show)

See Also:
Host:
Target: hppa*-*-{linux,hpux}
Build:
Known to work: 3.3.5
Known to fail: 4.0.0
Last reconfirmed: 2005-06-14 00:17:58


Attachments
Proposed patch (1.23 KB, patch)
2005-06-24 04:20 UTC, Mark Mitchell
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Randolph Chung 2005-06-13 16:14:25 UTC
with the following test program, gcc-4.0 doesn't emit the required function
canonicalization code (calls to __canonicalize_funcptr_for_compare) to do the
comparison when optimization is enabled.

tausq@riot:/tmp/dl$ gcc-3.3 -D_GNU_SOURCE -O2 -o default default.c -ldl;
./default; echo $?
0

tausq@riot:/tmp/dl$ gcc-4.0 -D_GNU_SOURCE -O2 -o default default.c -ldl;
./default; echo $?
1


#include <dlfcn.h>
#include <stdio.h>

int
main (int argc, char *argv[])
{
  void *p;

  /* Find function `main'.  */
  p = dlsym (RTLD_DEFAULT, "main");
  if (p == NULL)
    return -1;
  else if ((int (*)(int, char **))p != main)
    return 1;
  else
    return 0;
 }
Comment 1 Andrew Pinski 2005-06-13 16:20:05 UTC
I thought this was fixed.
Comment 2 Randolph Chung 2005-06-13 16:23:55 UTC
this is what you get with gcc-3.3:

.LC0:
.stringz"main"
        .section        .rodata.cst4,"aM",@progbits,4
        .align 4
.LC1:
        .word   P%main
[...]
        ldil LR'.LC0,%r25
        ldi -1,%r4
        bl dlsym,%r2
        ldo RR'.LC0(%r25),%r25
        comib,= 0,%r28,.L1
        copy %r28,%r26
        bl __canonicalize_funcptr_for_compare,%r2
        nop
        ldil LR'.LC1,%r19
        copy %r28,%r4
        bl __canonicalize_funcptr_for_compare,%r2
        ldw RR'.LC1(%r19),%r26
        comclr,= %r28,%r4,%r4
        ldi 1,%r4
.L1:
        copy %r4,%r28


And with gcc-4.0, you get instead:
.LC0:
.stringz"main"
        .section        .rodata.cst4,"aM",@progbits,4
        .align 4
.LC1:
        .word   P%main
[...]
        ldil LR'.LC0,%r25
        bl dlsym,%r2
        ldo RR'.LC0(%r25),%r25
        movb,= %r28,%r20,.L4
        ldi -1,%r19
        ldil LR'.LC1,%r28
        ldw RR'.LC1(%r28),%r19
        comclr,= %r19,%r20,%r19
        ldi 1,%r19
.L4:
        copy %r19,%r28

Comment 3 randolph 2005-06-13 16:24:51 UTC
Subject: Re:  [4.0/4.1 regression] Wrong code for function pointer comparision during optimization

> I thought this was fixed.

Nope, sorry.

randolph
Comment 4 John David Anglin 2005-06-14 00:14:39 UTC
Here's what we have at the test to see if function pointer canonicalization
should be done in dojump.c:

Breakpoint 1, do_compare_and_jump (exp=0x400ec708, signed_code=NE,
    unsigned_code=NE, if_false_label=0x400ecd70, if_true_label=0x0)
    at ../../gcc/gcc/dojump.c:928
928       if (HAVE_canonicalize_funcptr_for_compare
(gdb) p debug_tree (exp)
 <ne_expr 0x400ec708
    type <boolean_type 0x4000c620 _Bool public unsigned QI
        size <integer_cst 0x400030f0 constant invariant 8>
        unit size <integer_cst 0x40003108 constant invariant 1>
        align 8 symtab 0 alias set -1 precision 1 min <integer_cst 0x400034e0 0> 
max <integer_cst 0x40003510 1>>

    arg 0 <var_decl 0x400ef850 p
        type <pointer_type 0x400174d0 type <void_type 0x40017460 void>
            sizes-gimplified public unsigned SI
            size <integer_cst 0x400032e8 constant invariant 32>
            unit size <integer_cst 0x40003078 constant invariant 4>
            align 32 symtab 0 alias set -1
            pointer_to_this <pointer_type 0x4007fbd0>>
        used unsigned SI file cffc-bug.c line 8 size <integer_cst 0x400032e8 32> 
unit size <integer_cst 0x40003078 4>
        align 32 context <function_decl 0x400ef700 main>
        (reg/v/f:SI 94 [ p ])
        chain <var_decl 0x400ef8c0 result type <integer_type 0x4000c380 int>
            SI file cffc-bug.c line 9 size <integer_cst 0x400032e8 32> unit size 
<integer_cst 0x40003078 4>
            align 32 context <function_decl 0x400ef700 main>>>
    arg 1 <addr_expr 0x400eb820
        type <pointer_type 0x400f3070 type <function_type 0x400ef690>
            unsigned SI size <integer_cst 0x400032e8 32> unit size <integer_cst 
0x40003078 4>
            align 32 symtab 0 alias set -1>
        constant invariant
        arg 0 <function_decl 0x400ef700 main type <function_type 0x400ef690>
            addressable used public static decl_5 SI file cffc-bug.c line 7 
arguments <parm_decl 0x400ef540 argc> result <result_decl 0x400ef770 D.2328> 
initial <block 0x400f0b98>
            (mem:SI (symbol_ref/v:SI ("@main") <function_decl 0x400ef700 main>) 
[0 S4 A32])
            saved-insns 0x400de800>>>
$3 = void

The "(int (*)(int, char **))" cast appears to have been dropped from the
comparison.

I also believe that casts to int types are also being dropped when applied
to function pointer causing the opposite problem (unnecessary canonicalization).

In cffc-bug.c.t19.dce1, we have:

<L1>:;
  p___23_4 = (int (*<T4ef>) (int, char * *)) p_3;
  if (p___23_4 != main) goto <L2>; else goto <L3>;

In cffc-bug.c.t20.dom1, we have:

<L1>:;
  p___23_4 = (int (*<T4ef>) (int, char * *)) p_2;
  if (p_2 != main) goto <L2>; else goto <L3>;
Comment 5 Andrew Pinski 2005-06-14 00:17:58 UTC
Confirmed.
Comment 6 John David Anglin 2005-06-14 00:53:15 UTC
The same problem should apply to hppa2.0w-hp-hpux11.11 as it also requires
function pointer canonicalization.
Comment 7 Mark Mitchell 2005-06-24 04:20:42 UTC
Created attachment 9139 [details]
Proposed patch
Comment 8 Mark Mitchell 2005-06-24 13:23:55 UTC
The proposed patch has passed testing on x86_64-unknown-linux-gnu.
Comment 9 Diego Novillo 2005-06-24 14:11:46 UTC
(In reply to comment #7)
> Created an attachment (id=9139)
> Proposed patch
> 
This is fine.


Diego.
Comment 10 randolph 2005-06-24 14:23:56 UTC
Subject: Re:  [4.0/4.1 regression] Wrong code for function pointer comparision during optimization

> The proposed patch has passed testing on x86_64-unknown-linux-gnu.

confirmed on hppa-linux. works on original test case.

thanks,
randolph
Comment 11 Jeffrey A. Law 2005-06-24 16:36:51 UTC
Subject: Re:  [4.0/4.1 regression] Wrong code
	for function pointer comparision during optimization

On Fri, 2005-06-24 at 04:20 +0000, mmitchel at gcc dot gnu dot org
wrote:
> ------- Additional Comments From mmitchel at gcc dot gnu dot org  2005-06-24 04:20 -------
> Created an attachment (id=9139)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=9139&action=view)
> Proposed patch
That's really not the right solution.

Checking for a useless conversion is *way* too pessimistic.   This
code's sole purpose is to optimize away casting one pointer type to
another, then comparing the result against zero.  The only time that
is not safe is when the pointer requires canonicalization, which is
only an issue for function pointers.

The right way to deal with this is to reject the optimization when
presented with a function pointer.


[ Yes, the code is written more generally, but with the recent
  VRP improvements, I'm pretty sure the generality is no longer
  needed.  Given a tree combiner, this code will eventually
  disappear. ]





jeff


Comment 12 dave 2005-06-27 02:53:55 UTC
Subject: Re:  [4.0/4.1 regression] Wrong code for function pointer comparision during optimization

>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>  GCC target triplet|hppa-*-linux, hppa2.0w-hp-  |hppa*-*-{linux,hpux}
>                    |hpux11.11                   |

This problem doesn't affect hppa64 as it doesn't require function
pointer canonicalization.

Dave
Comment 13 Jeffrey A. Law 2005-07-01 19:04:46 UTC
Fixed by this patch:

http://gcc.gnu.org/ml/gcc-patches/2005-06/msg02278.html