[Bug lto/98971] New: LTO removes __patchable_function_entries

[Bug lto/98971] New: LTO removes __patchable_function_entries

[gabriel at inconstante dot net.br]

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

            Bug ID: 98971
           Summary: LTO removes __patchable_function_entries
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gabriel at inconstante dot net.br
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

Building with -flto removes the effects of -fpatchable-function-entries, or at
least some of them.

For instance, building the following code:

$ cat libtesta.c
int
testa7(void)
{
  return 7;
}

with:

$ gcc libtesta.c -fPIC -fpatchable-function-entry=4,2 -flto -c -o libtesta.o
$ gcc libtesta.o -flto -shared -o libtesta.so

removes the nop padding usually generated with -fpatchable-function-entry, as
well as it removes the __patchable_function_entries section from the resulting
binaries. The intermediate libtesta.o already lacks both:

$ readelf --sections libtesta.o | grep __patchable
$ readelf --sections libtesta.so | grep __patchable
$ objdump -d libtesta.so | grep "<testa7>:" -C3
00000000000010f0 <frame_dummy>:
    10f0:       e9 7b ff ff ff          jmpq   1070 <register_tm_clones>

00000000000010f5 <testa7>:
    10f5:       55                      push   %rbp
    10f6:       48 89 e5                mov    %rsp,%rbp
    10f9:       b8 07 00 00 00          mov    $0x7,%eax

Without -flto, I get what I expected:

$ readelf --sections libtesta.o | grep __patchable
  [ 4] __patchable_[...] PROGBITS         0000000000000000  00000050
$ readelf --sections libtesta.so | grep __patchable
  [19] __patchable_[...] PROGBITS         0000000000004020  00003020
$ objdump -d libtesta.so | grep "<testa7>:" -C3
    10f5:       90                      nop
    10f6:       90                      nop

00000000000010f7 <testa7>:
    10f7:       90                      nop
    10f8:       90                      nop
    10f9:       55                      push   %rbp

Building with a single gcc command, such as:

$ gcc libtesta.c -fPIC -fpatchable-function-entry=4,2 -flto -shared -o
libtesta.so

also works as I expected, i.e.: the nops and the __patchable_function_entries
are kept.


Is this the intended behavior? Am I using it wrong? Or is this a bug?

Cheers

PS: Tested with the branch for gcc 10, with trunk, and with the compilers from
openSUSE and Debian.

[Bug lto/98971] LTO removes __patchable_function_entries

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-02-05

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Thank you for the report, I'll take a look.

[Bug lto/98971] LTO removes __patchable_function_entries

[marxin at gcc dot gnu.org]

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 50133
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50133&action=edit
Tentative patch

As seen the flag -fpatchable-function-entry is properly marked as Optimization.
However, it's the argument is parsed early and stored into the following tuple:

; How many NOP insns to place at each function entry by default
Variable
HOST_WIDE_INT function_entry_patch_area_size

; And how far the real asm entry point is into this area
Variable
HOST_WIDE_INT function_entry_patch_area_start

That does not work with set_current_function where per-function arguments are
restored. My tentative patch fixes that.

The following examples works now:

$ cat pr98971.c
int
testa7(void)
{
  return 7;
}

int
__attribute__((patchable_function_entry(10,5)))
testa77(void)
{
  return 77;
}

#pragma GCC optimize("patchable-function-entry=0,0")

int
testa_no(void)
{
  return 1234;
}

$ cat pr98971-2.c
int
testa8(void)
{
  return 8;
}

$ gcc pr98971.c -fPIC -fpatchable-function-entry=4,1 -flto -c
$ gcc pr98971-2.c -fPIC -flto -c
$ gcc pr98971.o pr98971-2.o -flto -shared -o x.so
$ objdump -d x.so
...
0000000000000650 <frame_dummy>:
 650:   f3 0f 1e fa             endbr64 
 654:   e9 77 ff ff ff          jmp    5d0 <register_tm_clones>
 659:   90                      nop

000000000000065a <testa7>:
 65a:   90                      nop
 65b:   90                      nop
 65c:   90                      nop
 65d:   55                      push   %rbp
 65e:   48 89 e5                mov    %rsp,%rbp
 661:   b8 07 00 00 00          mov    $0x7,%eax
 666:   5d                      pop    %rbp
 667:   c3                      ret    
 668:   90                      nop
 669:   90                      nop
 66a:   90                      nop
 66b:   90                      nop
 66c:   90                      nop

000000000000066d <testa77>:
 66d:   90                      nop
 66e:   90                      nop
 66f:   90                      nop
 670:   90                      nop
 671:   90                      nop
 672:   55                      push   %rbp
 673:   48 89 e5                mov    %rsp,%rbp
 676:   b8 4d 00 00 00          mov    $0x4d,%eax
 67b:   5d                      pop    %rbp
 67c:   c3                      ret    

000000000000067d <testa_no>:
 67d:   55                      push   %rbp
 67e:   48 89 e5                mov    %rsp,%rbp
 681:   b8 d2 04 00 00          mov    $0x4d2,%eax
 686:   5d                      pop    %rbp
 687:   c3                      ret    

0000000000000688 <testa8>:
 688:   55                      push   %rbp
 689:   48 89 e5                mov    %rsp,%rbp
 68c:   b8 08 00 00 00          mov    $0x8,%eax
 691:   5d                      pop    %rbp
 692:   c3                      ret    

@Gabriel: Is it intended behavior?

[Bug lto/98971] LTO removes __patchable_function_entries

[gagomes at suse dot de]

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

--- Comment #3 from Gabriel F. T. Gomes <gagomes at suse dot de> ---
(In reply to Martin Liška from comment #2)
>
> @Gabriel: Is it intended behavior?

That's what I expected, yes! Thank you.

The only difference now is that the intermediate object doesn't have a
__patchable_function_entries section, but that's OK as far as I can tell.

With:

$ gcc libtesta.c -fPIC -fpatchable-function-entry=4,2 -flto -c -o libtesta.o
$ gcc libtesta.o -flto -shared -o libtesta.so

now I get:

$ readelf --sections libtesta.o | grep __patchable
$ readelf --sections libtesta.so | grep __patchable
  [22] __patchable_[...] PROGBITS         0000000000004020  00003020

Cheers!

[Bug lto/98971] LTO removes __patchable_function_entries

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
> The only difference now is that the intermediate object doesn't have a
> __patchable_function_entries section, but that's OK as far as I can tell.

Well, the intermediate object contains just LTO bytecode, that's why you can't
see the section. You can use -ffat-lto-objects in order to generate both
assembly and LTO bytecode.

[Bug lto/98971] LTO removes __patchable_function_entries

[gagomes at suse dot de]

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

--- Comment #5 from Gabriel F. T. Gomes <gagomes at suse dot de> ---
(In reply to Martin Liška from comment #4)
>
> Well, the intermediate object contains just LTO bytecode, that's why you
> can't see the section. You can use -ffat-lto-objects in order to generate
> both assembly and LTO bytecode.

Indeed. Thanks again! :)

[Bug lto/98971] LTO removes __patchable_function_entries

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:0d701e3eb89870237669ef7bf41394d90c35ae70

commit r11-7133-g0d701e3eb89870237669ef7bf41394d90c35ae70
Author: Martin Liska <mliska@suse.cz>
Date:   Fri Feb 5 13:11:44 2021 +0100

    opts: fix handling of -fpatchable-function-entries option

    gcc/ChangeLog:

            PR lto/98971
            * cfgexpand.c (pass_expand::execute): Parse per-function option
            flag_patchable_function_entry and use it.
            * common.opt: Remove function_entry_patch_area_size and
            function_entry_patch_area_start global variables.
            * opts.c (parse_and_check_patch_area): New function.
            (common_handle_option): Use it.
            * opts.h (parse_and_check_patch_area): New function.
            * toplev.c (process_options): Parse and use
            function_entry_patch_area_size.

[Bug lto/98971] LTO removes __patchable_function_entries

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.0
      Known to fail|11.0                        |

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
Fixed on master so far.
@Gabriel: Are you interested in backporting the patch?

[Bug lto/98971] LTO removes __patchable_function_entries

[marxin at gcc dot gnu.org]

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

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

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
So backporting to GCC-10 is not so easy. We would need
g:6607bdd99994c834f92fce924abdaea3405f62dc (aka PR93492).
That said, I tend not to backport it.