[Bug c++/105169] New: Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[Bug c++/105169] New: Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[giuliano.belinassi at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

            Bug ID: 105169
           Summary: Compiling C++ code with
                    -fpatchable-function-entry=16,14 results in references
                    to discarded sections
           Product: gcc
           Version: 11.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: giuliano.belinassi at gmail dot com
  Target Milestone: ---

Created attachment 52751
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52751&action=edit
Small project to reproduce claimed failure.

Compiling the attached project running `make` results in the following messages
from ld (GNU ld (GNU Binutils; SUSE Linux Enterprise 15) 2.37.20211103-7.26):

`.text._ZN21WinsockInterfaceClass12Get_ProtocolEv' referenced in section
`__patchable_function_entries' of wsproto.o: defined in discarded section
`.text._ZN21WinsockInterfaceClass12Get_ProtocolEv[_ZN21WinsockInterfaceClass12Get_ProtocolEv]'
of wsproto.o

The project compiles fine without -fpatchable-function-entry.

PS: I could not manage to reduce the testcase into a single file.

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Keywords|                            |link-failure
      Known to fail|                            |11.2.1, 12.0, 7.5.0
                 CC|                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-04-06

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
wspudp.cpp is not used?  Confirmed.

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed, a bit reduced test-case:

$ cat 1.ii
struct WinsockInterfaceClass {
  virtual int Get_Protocol() { return 0; }
} PacketTransport;

$ cat 2.ii
struct WinsockInterfaceClass {
  WinsockInterfaceClass();
  virtual void Set_Broadcast_Address() {}
  virtual int Get_Protocol() { return 0; }
};
WinsockInterfaceClass::WinsockInterfaceClass() {}

int main()
{
  return 0;
}

$ g++ 1.ii 2.ii -fpatchable-function-entry=1 -O2
`.text._ZN21WinsockInterfaceClass12Get_ProtocolEv' referenced in section
`__patchable_function_entries' of /tmp/cc6P6bkZ.o: defined in discarded section
`.text._ZN21WinsockInterfaceClass12Get_ProtocolEv[_ZN21WinsockInterfaceClass12Get_ProtocolEv]'
of /tmp/cc6P6bkZ.o
collect2: error: ld returned 1 exit status

$ g++ 1.ii 2.ii -fpatchable-function-entry=1 -O2 -fuse-ld=gold
/tmp/ccCrCzcR.o(__patchable_function_entries+0x8): error: relocation refers to
local symbol "" [4], which is defined in a discarded section
  section group signature: "_ZN21WinsockInterfaceClass12Get_ProtocolEv"
  prevailing definition is from /tmp/ccH1NIH4.o
collect2: error: ld returned 1 exit status

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
And the following works:

g++ 2.ii 1.ii -fpatchable-function-entry=1 -O2

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #2)
> Confirmed, a bit reduced test-case:
> 
> $ cat 1.ii
> struct WinsockInterfaceClass {
>   virtual int Get_Protocol() { return 0; }
> } PacketTransport;
> 
> $ cat 2.ii
> struct WinsockInterfaceClass {
>   WinsockInterfaceClass();
>   virtual void Set_Broadcast_Address() {}
>   virtual int Get_Protocol() { return 0; }
> };

Note this is a ODR violation, so better match up both WinsockInterfaceClass
types.

> WinsockInterfaceClass::WinsockInterfaceClass() {}
> 
> int main()
> {
>   return 0;
> }
> 
> $ g++ 1.ii 2.ii -fpatchable-function-entry=1 -O2
> `.text._ZN21WinsockInterfaceClass12Get_ProtocolEv' referenced in section
> `__patchable_function_entries' of /tmp/cc6P6bkZ.o: defined in discarded
> section
> `.text.
> _ZN21WinsockInterfaceClass12Get_ProtocolEv[_ZN21WinsockInterfaceClass12Get_Pr
> otocolEv]' of /tmp/cc6P6bkZ.o
> collect2: error: ld returned 1 exit status
> 
> $ g++ 1.ii 2.ii -fpatchable-function-entry=1 -O2 -fuse-ld=gold
> /tmp/ccCrCzcR.o(__patchable_function_entries+0x8): error: relocation refers
> to local symbol "" [4], which is defined in a discarded section
>   section group signature: "_ZN21WinsockInterfaceClass12Get_ProtocolEv"
>   prevailing definition is from /tmp/ccH1NIH4.o
> collect2: error: ld returned 1 exit status

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
It's there since the introduction of the option in r8-2176-g417ca0117a1a9a8a.

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 6 Apr 2022, marxin at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169
> 
> --- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
> It's there since the introduction of the option in r8-2176-g417ca0117a1a9a8a.

Yes, I think it's a fundamental limitation of how we emit refereces into
the section if the defs are inside comdats - the refs are locally
resolved instead of being symbolic.  But IMHO somehow amending the
patchable functions list from inside the comdat group itself would
be preferable.

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
Yes, looking at clang++, they utilize G (group) section flags:

_ZN21WinsockInterfaceClass12Get_ProtocolEv: #
@_ZN21WinsockInterfaceClass12Get_ProtocolEv
.Lfunc_begin0:
        .cfi_startproc
# %bb.0:
        nop
        xorl    %eax, %eax
        retq
.Lfunc_end0:
        .size   _ZN21WinsockInterfaceClass12Get_ProtocolEv,
.Lfunc_end0-_ZN21WinsockInterfaceClass12Get_ProtocolEv
        .cfi_endproc
        .section       
__patchable_function_entries,"aGwo",@progbits,_ZN21WinsockInterfaceClass12Get_ProtocolEv,comdat,_ZN21WinsockInterfaceClass12Get_ProtocolEv
        .p2align        3
        .quad   .Lfunc_begin0
                                        # -- End function

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[matz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

Michael Matz <matz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at gcc dot gnu.org

--- Comment #8 from Michael Matz <matz at gcc dot gnu.org> ---
Something like this would be needed.  It's proof-of-concept.  It actually just
copy-pastes code from default_function_rodata_section without proper
integration
and caring for other cases handled there.  To be done properly it would need
abstracting what default_function_rodata_section does, just with a choosable
prefix.  Additionally the whole section dealing in GCC should be changed such
that internally not the section name is the only differentiator for the section
hash table: currently that's the reason the comdat name has to be appended
to the section name, although ELF doesn't need this for comdat sections.  So,
terrible hack, but ...

diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index c9b5208853d..1ca5a4c3592 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1963,14 +1963,36 @@ default_print_patchable_function_entry_1 (FILE *file,
       char buf[256];
       static int patch_area_number;
       section *previous_section = in_section;
+      section *sect;
       const char *asm_op = integer_asm_op (POINTER_SIZE_UNITS, false);

       gcc_assert (asm_op != NULL);
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);

-      switch_to_section (get_section ("__patchable_function_entries",
-                                     flags, current_function_decl));
+      if (DECL_COMDAT_GROUP (current_function_decl) && HAVE_COMDAT_GROUP)
+       {
+         const char *dot;
+         size_t len;
+         char* rname;
+         const char *sname = "__patchable_function_entries";
+         const char *name = DECL_SECTION_NAME (current_function_decl);
+
+         dot = strchr (name + 1, '.');
+         if (!dot)
+           dot = name;
+         len = strlen (dot) + strlen (sname) + 1;
+         rname = (char *) alloca (len);
+
+         strcpy (rname, sname);
+         strcat (rname, dot);
+         sect = get_section (rname, (SECTION_LINKONCE | flags),
+                             current_function_decl);
+       }
+      else
+       sect = get_section ("__patchable_function_entries", flags,
+                           current_function_decl);
+      switch_to_section (sect);
       assemble_align (POINTER_SIZE);
       fputs (asm_op, file);
       assemble_name_raw (file, buf);

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
> +         const char *sname = "__patchable_function_entries";
> +         const char *name = DECL_SECTION_NAME (current_function_decl);
> +
> +         dot = strchr (name + 1, '.');
> +         if (!dot)
> +           dot = name;
> +         len = strlen (dot) + strlen (sname) + 1;
> +         rname = (char *) alloca (len);
> +
> +         strcpy (rname, sname);
> +         strcat (rname, dot);

I would prefer using concat function, so something like:

          char *rname
            = concat ("__patchable_function_entries", dot, NULL);
          sect = get_section (rname, (SECTION_LINKONCE | flags),
                              current_function_decl);
          free (rname);

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, a good example might be how we handle .vtable_map_vars for VTV which
uses handle_vtv_comdat_section instead of switch_to_section.  It might have
more specialities but then it should serve as a recipie how to handle
non-ELF.

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #10)
> Btw, a good example might be how we handle .vtable_map_vars for VTV which
> uses handle_vtv_comdat_section instead of switch_to_section.  It might have
> more specialities but then it should serve as a recipie how to handle
> non-ELF.

So sth like the following would do the trick for ELF.  But maybe better
abstract a switch_to_comdat_section and re-use that from the vtv handler
and this place to avoid touching in_section directly.

diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index e22bc66a6c8..632c9486ccb 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -1995,8 +1995,13 @@ default_print_patchable_function_entry_1 (FILE *file,
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);

-      switch_to_section (get_section ("__patchable_function_entries",
-                                     flags, current_function_decl));
+      section *sect = get_section ("__patchable_function_entries",
+                                  flags, current_function_decl);
+      targetm.asm_out.named_section  (sect->named.name,
+                                     sect->named.common.flags
+                                     | SECTION_LINKONCE,
+                                     current_function_decl);
+      in_section = sect;
       assemble_align (POINTER_SIZE);
       fputs (asm_op, file);
       assemble_name_raw (file, buf);

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #11)
> (In reply to Richard Biener from comment #10)
> > Btw, a good example might be how we handle .vtable_map_vars for VTV which
> > uses handle_vtv_comdat_section instead of switch_to_section.  It might have
> > more specialities but then it should serve as a recipie how to handle
> > non-ELF.
> 
> So sth like the following would do the trick for ELF.  But maybe better
> abstract a switch_to_comdat_section and re-use that from the vtv handler
> and this place to avoid touching in_section directly.

switch_to_section already takes a 'decl' as argument but that's only used
for some diagnostics.

OTOH it's a bit odd to switch to the "same" __patchable_function_entries
section but each time associate it with another decl.  A nice API will
ask for quite some bikeshedding I suppose.

So maybe just handle_pfe_comdat_section ...

[Bug c++/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Giuliano Belinassi
<giulianob@gcc.gnu.org>:

https://gcc.gnu.org/g:7a3f38a966a52893fb5bae301a1a3d56961358fb

commit r13-566-g7a3f38a966a52893fb5bae301a1a3d56961358fb
Author: Giuliano Belinassi <gbelinassi@suse.de>
Date:   Fri May 6 23:37:52 2022 -0300

    PR105169 Fix references to discarded sections

    When -fpatchable-function-entry= is enabled, certain C++ codes fails to
    link because of generated references to discarded sections in
    __patchable_function_entry section. This commit fixes this problem by
    puting those references in a COMDAT section.

    2022-05-06  Giuliano Belinassi  <gbelinassi@suse.de>

    gcc/ChangeLog
            PR c++/105169
            * targhooks.cc (default_print_patchable_function_entry_1): Handle
COMDAT case.
            * varasm.cc (switch_to_comdat_section): New
            (handle_vtv_comdat_section): Call switch_to_comdat_section.
            * varasm.h: Declare switch_to_comdat_section.

    gcc/testsuite/ChangeLog
    2022-05-06  Giuliano Belinassi  <gbelinassi@suse.de>

            PR c++/105169
            * g++.dg/modules/pr105169.h: New file.
            * g++.dg/modules/pr105169_a.C: New test.
            * g++.dg/modules/pr105169_b.C: New file.

[Bug middle-end/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=110729
                 CC|                            |i at maskray dot me

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110729 has been marked as a duplicate of this bug. ***

[Bug middle-end/105169] Compiling C++ code with -fpatchable-function-entry=16,14 results in references to discarded sections

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105169

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |13.0
         Resolution|---                         |FIXED

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.