Bug 28157

Summary: gas: .symver *, *@*, remove cannot be used in relocation
Product: binutils Reporter: Fangrui Song <i>
Component: gasAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: hjl.tools, mliska
Priority: P2    
Version: 2.38 (HEAD)   
Target Milestone: 2.38   
Host: Target:
Build: Last reconfirmed: 2021-08-01 00:00:00
Attachments: A patch

Description Fangrui Song 2021-07-31 20:07:40 UTC
PR 25295 added the new syntax `, remove` to `.symver`.

However, the new syntax cannot be used in relocations. This makes it difficult to use:

% cat ctype_info.s
        .symver __ctype_b,__ctype_b@GLIBC_2.2.5, remove

# These directives work
        .data
        .globl  __ctype_b
        .type   __ctype_b, @object
        .size   __ctype_b, 8

__ctype_b:
# Error: redefined symbol cannot be used on reloc
        .quad   __ctype_b

% as ctype_info.s       
ctype_info.s: Assembler messages:
ctype_info.s:11: Error: redefined symbol cannot be used on reloc


Personally I think `.symver a, b@ver, remove` should behave like "rename a to b@ver". All relocations should be relative the versioned b@ver instead of a.
Comment 1 H.J. Lu 2021-08-01 00:02:48 UTC
Created attachment 13571 [details]
A patch

Please try this.
Comment 2 Fangrui Song 2021-08-07 06:29:35 UTC
(In reply to H.J. Lu from comment #1)
> Created attachment 13571 [details]
> A patch
> 
> Please try this.

I applied this patch.

% cat /tmp/c/y.s
call __free_hook
.symver __free_hook, __free_hook@GLIBC.2.2.5, remove
# for an undefined symbol, I believe `remove` is a no-op.

% gas/as-new /tmp/c/y.s -o y.o
/tmp/c/y.s: Assembler messages:
/tmp/c/y.s: Internal error (Segmentation fault).
Please report this bug.


I don't know the underlying implementation but I wonder whether the defined single-@ `remove` case can share the same code with defined `@@@`. `@@ remove` is identical to `@@@`.
Comment 3 Fangrui Song 2021-08-07 06:31:59 UTC
For defined non-`remove` single-@, you may want to change the relocations to reference the versioned symbol as well.

Then the only difference between defined `remove` single-@ and defined non-`remove` single-@ is whether the unadorned symbol is emitted.

Personally I really hope that we can just default to `remove`.
Comment 4 H.J. Lu 2021-08-07 13:02:23 UTC
(In reply to Fangrui Song from comment #2)
> (In reply to H.J. Lu from comment #1)
> > Created attachment 13571 [details]
> > A patch
> > 
> > Please try this.
> 
> I applied this patch.
> 
> % cat /tmp/c/y.s
> call __free_hook
> .symver __free_hook, __free_hook@GLIBC.2.2.5, remove
> # for an undefined symbol, I believe `remove` is a no-op.
> 
> % gas/as-new /tmp/c/y.s -o y.o
> /tmp/c/y.s: Assembler messages:
> /tmp/c/y.s: Internal error (Segmentation fault).
> Please report this bug.

Works for me:

[hjl@gnu-cfl-2 pr28157]$ cat foo.s
	call __free_hook
	.symver __free_hook, __free_hook@GLIBC.2.2.5, remove
[hjl@gnu-cfl-2 pr28157]$ ./as foo.s -o foo.o
[hjl@gnu-cfl-2 pr28157]$ objdump -dwr foo.o

foo.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <.text>:
   0:	e8 00 00 00 00       	call   0x5	1: R_X86_64_PLT32	__free_hook@GLIBC.2.2.5-0x4
[hjl@gnu-cfl-2 pr28157]$ nm foo.o
                 U __free_hook@GLIBC.2.2.5
[hjl@gnu-cfl-2 pr28157]$
Comment 5 H.J. Lu 2021-08-07 13:06:06 UTC
The current patch is at

https://sourceware.org/pipermail/binutils/2021-August/117562.html
Comment 6 cvs-commit@gcc.gnu.org 2021-08-16 13:50:01 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=eb09df162bafa67abee713be594a99bd20bd6825

commit eb09df162bafa67abee713be594a99bd20bd6825
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Aug 16 06:46:44 2021 -0700

    as: Replace the removed symbol with the versioned symbol
    
    When a symbol removed by .symver is used in relocation and there is one
    and only one versioned symbol, don't remove the symbol.  Instead, mark
    it to be removed and replace the removed symbol used in relocation with
    the versioned symbol before generating relocation.
    
            PR gas/28157
            * symbols.c (symbol_flags): Add removed.
            (symbol_entry_find): Updated.
            (symbol_mark_removed): New function.
            (symbol_removed_p): Likewise.
            * symbols.h (symbol_mark_removed): New prototype.
            (symbol_removed_p): Likewise.
            * write.c (write_relocs): Call obj_fixup_removed_symbol on
            removed fixp->fx_addsy and fixp->fx_subsy if defined.
            (set_symtab): Don't add a symbol if symbol_removed_p returns true.
            * config/obj-elf.c (elf_frob_symbol):  Don't remove the symbol
            if it is used on relocation.  Instead, mark it as to be removed
            and issue an error if the symbol has more than one versioned name.
            (elf_fixup_removed_symbol): New function.
            * config/obj-elf.h (elf_fixup_removed_symbol): New prototype.
            (obj_fixup_removed_symbol): New.
            * testsuite/gas/symver/symver11.d: Updated expected error
            message.
            * testsuite/gas/symver/symver16.d: New file.
            * testsuite/gas/symver/symver16.s: Likewise.
Comment 7 H.J. Lu 2021-08-16 13:54:18 UTC
Fixed for 2.38.
Comment 8 cvs-commit@gcc.gnu.org 2021-08-18 04:11:36 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a86733d63d40fd8d3a7a2640ebcc048414fb465f

commit a86733d63d40fd8d3a7a2640ebcc048414fb465f
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Aug 18 13:21:33 2021 +0930

    Re: as: Replace the removed symbol with the versioned symbol
    
    Some targets, typically embedded without shared libraries, replace the
    relocation symbol with a section symbol (see tc_fix_adjustable).
    Allow the test to pass for such targets.  Fixes the following.
    
    avr-elf  +FAIL: symver symver16
    d10v-elf  +FAIL: symver symver16
    dlx-elf  +FAIL: symver symver16
    ip2k-elf  +FAIL: symver symver16
    m68k-elf  +FAIL: symver symver16
    mcore-elf  +FAIL: symver symver16
    pj-elf  +FAIL: symver symver16
    s12z-elf  +FAIL: symver symver16
    visium-elf  +FAIL: symver symver16
    z80-elf  +FAIL: symver symver16
    
            PR gas/28157
            * testsuite/gas/symver/symver16.d: Relax reloc match.