[Bug c/57260] New: Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[Bug c/57260] New: Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

            Bug ID: 57260
           Summary: Generated R_MIPS_GOT_MIPS relocation for direct
                    function call while compiling with -O2 on MIPS N64
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lee.duhem at gmail dot com

Created attachment 30101
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30101&action=edit
includes glxdri.c, glxdri.xgcc.O0.s and glxdri.xgcc.O2.s

If glxdri.c, which is part of Xorg source, is compiled by 
gcc with -O2, R_MIPS_GOT_DISP relocations will be generated 
for two direct function calls, DRIBlockHandler and DRIWakeupHandler.
While Xorg loads the libglx.so that is generated by using glxdri.o
and other object files, dlopen will give error message which says
'undefined symbol: DRIBlockHandler'. 

If glxdri.c is compiled by gcc with -O0, all the relocations
for both DRIBlockHandler and DRIWakeupHandler are R_MIPS_CALL16,
this time the libglx.so will be loaded correctly by dlopen.

In the -O2 case, is the relocation type for DRIBlockHandler and
DRIWakeupHandler correct?

Just for your information, here is the glxdir.c compiling command
(default ABI is MIPS N64):

~/gcc/build0/gcc/xgcc -B ~/gcc/build0/gcc/ -std=gnu99 -DHAVE_CONFIG_H -I.
-I../include -I../hw/xfree86/os-support -I../hw/xfree86/os-support/bus
-I../hw/xfree86/common -I../hw/xfree86/dri -I../mi -I../hw/xfree86/dri2
-DHAVE_DIX_CONFIG_H -Wall -Wpointer-arith -Wstrict-prototypes
-Wmissing-prototypes -Wmissing-declarations -Wnested-externs
-fno-strict-aliasing -Wbad-function-cast -Wformat=2 -Wold-style-definition
-Wdeclaration-after-statement -D_BSD_SOURCE -DHAS_FCHOWN -DHAS_STICKY_DIR_BIT
-I/usr/include/freetype2 -I/usr/include/pixman-1 -I../include -I../include
-I../Xext -I../composite -I../damageext -I../xfixes -I../Xi -I../mi
-I../miext/sync -I../miext/shadow -I../miext/damage -I../render -I../randr
-I../fb -fvisibility=hidden -I/usr/include/libdrm -I/usr/include/libdrm
-I/usr/include/libdrm -I/usr/include/X11/dri -DXFree86Server -DGLX_USE_TLS
-DPTHREADS -O2 -g -Wl,-z,relro -fno-strict-aliasing -MT glxdri.lo -MD -MP -MF
.deps/glxdri.Tpo -S glxdri.c  -fPIC -DPIC -o .libs/glxdri.xgcc.O2.s

[Bug c/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

--- Comment #1 from Lee Duhem <lee.duhem at gmail dot com> ---
Created attachment 30102
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30102&action=edit
the preprocessed glxdri.c

[Bug c/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

Lee Duhem <lee.duhem at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |4.9.0

--- Comment #2 from Lee Duhem <lee.duhem at gmail dot com> ---
$ ~/gcc/build0/gcc/xgcc -B ~/gcc/build0/gcc/ -v
Reading specs from /home/libin/gcc/build0/gcc/specs
COLLECT_GCC=/home/libin/gcc/build0/gcc/xgcc
COLLECT_LTO_WRAPPER=/home/libin/gcc/build0/gcc/lto-wrapper
Target: mips64el-neokylin-linux-gnu
Configured with: ../gcc/configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix
--enable-checking=release --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-gnu-unique-object --enable-languages=c
--disable-fixed-point --without-ppl --without-cloog --disable-checking
--disable-werror --host=mips64el-neokylin-linux-gnu
--build=mips64el-neokylin-linux-gnu --target=mips64el-neokylin-linux-gnu
--with-abi=64 --disable-bootstrap
Thread model: posix
gcc version 4.9.0 20130509 (experimental) (GCC)

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks like sibcalls are incorrectly done.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |normal

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> Looks like sibcalls are incorrectly done.

Or maybe not since it is no longer a call but rather a sibcall which means it
can never be using call16 here since it is not a real call. 

Smallest testcase:
int shouldbecall16(void);
int g(void) { return shouldbecall16(); }
---- CUT ---

        lui     $3,%hi(%neg(%gp_rel(g)))
        daddu   $3,$3,$25
        daddiu  $3,$3,%lo(%neg(%gp_rel(g)))
        ld      $25,%got_disp(shouldbecall16)($3)
        .reloc  1f,R_MIPS_JALR,shouldbecall16
1:      jr      $25
        nop


Notice it is a jr rather than a jalr.

I think this is correct behavior either way really.  I think Xorg's sources are
depending on undefined behavior dealing with undefined symbols.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

--- Comment #5 from Lee Duhem <lee.duhem at gmail dot com> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Andrew Pinski from comment #3)
> > Looks like sibcalls are incorrectly done.
> 
> Or maybe not since it is no longer a call but rather a sibcall which means
> it can never be using call16 here since it is not a real call. 
> 
> Smallest testcase:
> int shouldbecall16(void);
> int g(void) { return shouldbecall16(); }

I think the following testcase is closer to my case:
int shouldbecall16(void);
int g(void) { shouldbecall16(); return 0; }

And in this case, I got (with or without -O2):
    daddiu    $sp,$sp,-16
    sd    $28,0($sp)
    lui    $28,%hi(%neg(%gp_rel(g)))
    daddu    $28,$28,$25
    daddiu    $28,$28,%lo(%neg(%gp_rel(g)))
    ld    $25,%call16(shouldbecall16)($28)
    sd    $31,8($sp)
    .reloc    1f,R_MIPS_JALR,shouldbecall16
1:    jalr    $25
    nop


Thank you for your time.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Lee Duhem from comment #5)
> (In reply to Andrew Pinski from comment #4)
> > (In reply to Andrew Pinski from comment #3)
> > > Looks like sibcalls are incorrectly done.
> > 
> > Or maybe not since it is no longer a call but rather a sibcall which means
> > it can never be using call16 here since it is not a real call. 
> > 
> > Smallest testcase:
> > int shouldbecall16(void);
> > int g(void) { return shouldbecall16(); }
> 
> I think the following testcase is closer to my case:
> int shouldbecall16(void);
> int g(void) { shouldbecall16(); return 0; }

No it is not; mine is the one which shows the issue:
static void
__glXDRIenterServer(GLboolean rendering)
{
....
    DRIWakeupHandler(((void *)0), 0, ((void *)0));
}

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

--- Comment #7 from Lee Duhem <lee.duhem at gmail dot com> ---
(In reply to Andrew Pinski from comment #6)
> > I think the following testcase is closer to my case:
> > int shouldbecall16(void);
> > int g(void) { shouldbecall16(); return 0; }
> 
> No it is not; mine is the one which shows the issue:
> static void
> __glXDRIenterServer(GLboolean rendering)
> {
> ....
>     DRIWakeupHandler(((void *)0), 0, ((void *)0));
> }

There are two symbols that have the R_MIPS_GOT_DISP 
relocation, DRIBlockHandler and DRIWakeupHandler.

The DRIBlockHandler case is

static void
__glXDRIleaveServer(GLboolean rendering)
{
...

    DRIBlockHandler(NULL, NULL, NULL);

    for (i = 0; rendering && i < screenInfo.numScreens; i++) {
...
}

And the DRIWakeupHandler case is

static void
__glXDRIenterServer(GLboolean rendering)
{
...

    DRIWakeupHandler(NULL, 0, NULL);
}

Would you like to check again about the first case? I think
these two case are different.

Thank you for your time.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Lee Duhem from comment #7)
> Would you like to check again about the first case? I think
> these two case are different.

No they are both the same case:
    ld    $25,%got_disp(DRIWakeupHandler)($28)
    .loc 1 179 0
    ld    $28,32($sp)
    .loc 1 178 0
    move    $4,$0
    move    $5,$0
    move    $6,$0
    .cfi_restore 16
    .cfi_restore 17
    .cfi_restore 18
    .cfi_restore 28
    .cfi_restore 31
    .reloc    1f,R_MIPS_JALR,DRIWakeupHandler
1:    jr    $25
    daddiu    $sp,$sp,48
....
    ld    $25,%got_disp(DRIBlockHandler)($28)
    .loc 1 161 0
    ld    $28,64($sp)
    .loc 1 137 0
    move    $4,$0
    move    $5,$0
    move    $6,$0
    .cfi_restore 16
    .cfi_restore 17
    .cfi_restore 18
    .cfi_restore 19
    .cfi_restore 20
    .cfi_restore 21
    .cfi_restore 22
    .cfi_restore 28
    .cfi_restore 31
    .reloc    1f,R_MIPS_JALR,DRIBlockHandler
1:    jr    $25
    daddiu    $sp,$sp,80

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

--- Comment #9 from Lee Duhem <lee.duhem at gmail dot com> ---
(In reply to Andrew Pinski from comment #8)
> No they are both the same case:

If glxdri.c is compiled by gcc with -O2, gcc 4.9.0 will generate
four relocations for DRIBlockHandler, two R_MIPS_JALR, one R_MIPS_CALL16
and one R_MIPS_GOT_DISP; and gcc 4.4.6 will generate three relocations
for DRIBlockHandler, two R_MIPS_CALL16 and one R_MIPS_GOT_DISP.

According to "64-bit ELF Object File Specification"[1], the difference
between R_MIPS_CALL16 and R_MIPS_GOT_DISP is that the former could
be resolved to a lazy stub by the dynamic linker, "whereas the latter
must be resolved to the ultimate address at initialization." In other
words, lazy binding would not work for R_MIPS_GOT_DISP.

My question is:
1. Why does gcc generate R_MIPS_GOT_DISP for the calls to DRIBlockHandler
or DRIWakeupHandler?

2. Is it possible to modify glxdir.c so that in the final libglx.so, which
is generated by using glxdri.o and other object files, the calls to
DRIBlockHandler and DRIWakeupHandler could be resolved to a lazy stub,
therefore dlopen can open it correctly?

Thank you for your time.

[1]
http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-05-14
                 CC|                            |rsandifo at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Confirmed.

As discusseded on the binutils list, this is definitely a bug IMO.
It's a QoI requirement that we use lazy binding for direct calls.
I don't think I'd realised quite how important that is when
I implemented sibling calls for PIC way back when.

As for why GCC is using %got_disp for sibling calls: the problem
is that lazy-binding stubs require $gp to be valid on entry.
And $gp is call-saved on n32 and n64, so functions must restore
the _caller's_ value of $gp before returning from a function.
This means that $gp might hold the wrong value on entry to a
sibcalled stub.

I think the fix is simply to disable direct sibling calls for
n32 and n64 non-PLT abicalls.  (o32 is OK, because $gp is call-clobbered
there.)  It's unfortunate that we lose optimisation for a once-per-run
thing, but I don't think there's any alternative.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[rsandifo at gcc dot gnu.org]

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

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Created attachment 30116
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30116&action=edit
Candidate patch

Here's the patch I'm testing.  Lee, could you check that it
fixes the original libglx problem?

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[lee.duhem at gmail dot com]

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

--- Comment #12 from Lee Duhem <lee.duhem at gmail dot com> ---
(In reply to rsandifo@gcc.gnu.org from comment #11)
> Created attachment 30116 [details]
> Candidate patch
> 
> Here's the patch I'm testing.  Lee, could you check that it
> fixes the original libglx problem?

The libglx.so problem has been fixed by your patch, thank you.

But the regression tests are not finished yet, the MIPS machine
I am using quiet slow.

Thanks for your time.

[Bug target/57260] Generated R_MIPS_GOT_MIPS relocation for direct function call while compiling with -O2 on MIPS N64

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Thanks for the testing.  In the end I just used the
mips_function_ok_for_sibcall part of the patch I attached.
I later realised the other parts weren't correct.

Applied to trunk.