[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #13 from Jakub Jelinek --- I think -fpatchable-function-entry support is more than 3 years old now, so I think we can't treat it like a new feature and so I don't understand defering fixing it for stage1. Doesn't the kernel use that option too for quite some time?
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #14 from Alan Modra --- -fpatchable-function-entry isn't used by the powerpc linux kernel.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #15 from Jakub Jelinek --- In any case, if the option is totally broken, we should either fix it for GCC 11 or emit sorry message to make it clear to users the option is not supported.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #16 from Jakub Jelinek --- But the latter (i.e. sorry) only if it always has been broken for that target. #c7 seems to suggest it worked in ELFv1 (does it work on powerpc -m32?) and got broken by H.J's change, then we should fix it rather than stop supporting it. And if it has always been broken on powerpc64le, then sorry might be one of the options.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125
--- Comment #17 from Jakub Jelinek ---
As a quick hack, we could do e.g.
--- gcc/configure.ac2021-03-23 19:42:05.417907561 +0100
+++ gcc/configure.ac2021-03-30 14:54:31.655766205 +0200
@@ -3404,12 +3404,15 @@ AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_GNU_RETA
# Test if the assembler supports the section flag 'o' for specifying
# section with link-order.
case "${target}" in
- # Solaris may use GNU assembler with Solairs ld. Even if GNU
+ # Solaris may use GNU assembler with Solaris ld. Even if GNU
# assembler supports the section flag 'o', it doesn't mean that
# Solairs ld supports it.
*-*-solaris2*)
gcc_cv_as_section_link_order=no
;;
+ powerpc*-*-*)
+gcc_cv_as_section_link_order=no
+;;
*)
gcc_GAS_CHECK_FEATURE([section 'o' flag], gcc_cv_as_section_link_order,
[2,35,0], [--fatal-warnings],
i.e. effectively revert H.J's patch for powerpc* targets.
That is a hack rather than proper solution.
For GCC11 we need some solution at least for the P1 regression, i.e. the
ELFv1 support that worked in the past and was broken by
r11-5656-g694d4a6d0c466d0fbc97920a9c6641a7b349ca35
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125
--- Comment #18 from Jakub Jelinek ---
Another more targeted partial reversion:
2021-03-30 Jakub Jelinek
* targhooks.h (default_print_patchable_function_entry_1): Declare.
* targhooks.c (default_print_patchable_function_entry_1): New function,
copied from default_print_patchable_function_entry with an added flags
argument.
(default_print_patchable_function_entry): Rewritten into a small
wrapper around default_print_patchable_function_entry_1.
* config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
Redefine.
(rs6000_print_patchable_function_entry): New function.
--- gcc/targhooks.h.jj 2021-01-04 10:25:39.665224403 +0100
+++ gcc/targhooks.h 2021-03-30 15:48:42.826706369 +0200
@@ -230,6 +230,9 @@ extern bool default_use_by_pieces_infras
bool);
extern int default_compare_by_pieces_branch_ratio (machine_mode);
+extern void default_print_patchable_function_entry_1 (FILE *,
+ unsigned HOST_WIDE_INT,
+ bool, unsigned int);
extern void default_print_patchable_function_entry (FILE *,
unsigned HOST_WIDE_INT,
bool);
--- gcc/targhooks.c.jj 2021-01-04 10:25:38.974232228 +0100
+++ gcc/targhooks.c 2021-03-30 15:51:22.924932795 +0200
@@ -1832,17 +1832,15 @@ default_compare_by_pieces_branch_ratio (
return 1;
}
-/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
- entry. If RECORD_P is true and the target supports named sections,
- the location of the NOPs will be recorded in a special object section
- called "__patchable_function_entries". This routine may be called
- twice per function to put NOPs before and after the function
- entry. */
+/* Helper for default_print_patchable_function_entry and other
+ print_patchable_function_entry hook implementations. */
void
-default_print_patchable_function_entry (FILE *file,
- unsigned HOST_WIDE_INT patch_area_size,
- bool record_p)
+default_print_patchable_function_entry_1 (FILE *file,
+ unsigned HOST_WIDE_INT
+ patch_area_size,
+ bool record_p,
+ unsigned int flags)
{
const char *nop_templ = 0;
int code_num;
@@ -1864,9 +1862,6 @@ default_print_patchable_function_entry (
patch_area_number++;
ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
- unsigned int flags = SECTION_WRITE | SECTION_RELRO;
- if (HAVE_GAS_SECTION_LINK_ORDER)
- flags |= SECTION_LINK_ORDER;
switch_to_section (get_section ("__patchable_function_entries",
flags, current_function_decl));
assemble_align (POINTER_SIZE);
@@ -1883,6 +1878,25 @@ default_print_patchable_function_entry (
output_asm_insn (nop_templ, NULL);
}
+/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
+ entry. If RECORD_P is true and the target supports named sections,
+ the location of the NOPs will be recorded in a special object section
+ called "__patchable_function_entries". This routine may be called
+ twice per function to put NOPs before and after the function
+ entry. */
+
+void
+default_print_patchable_function_entry (FILE *file,
+ unsigned HOST_WIDE_INT patch_area_size,
+ bool record_p)
+{
+ unsigned int flags = SECTION_WRITE | SECTION_RELRO;
+ if (HAVE_GAS_SECTION_LINK_ORDER)
+flags |= SECTION_LINK_ORDER;
+ default_print_patchable_function_entry_1 (file, patch_area_size, record_p,
+ flags);
+}
+
bool
default_profile_before_prologue (void)
{
--- gcc/config/rs6000/rs6000.c.jj 2021-03-29 11:15:49.942202792 +0200
+++ gcc/config/rs6000/rs6000.c 2021-03-30 15:59:10.299755166 +0200
@@ -1341,6 +1341,9 @@ static const struct attribute_spec rs600
#define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
#endif
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
rs6000_print_patchable_function_entry
+
#undef TARGET_SET_UP_BY_PROLOGUE
#define TARGET_SET_UP_BY_PROLOGUE rs6000_set_up_by_prologue
@@ -14642,6 +14645,30 @@ rs6000_assemble_visibility (tree decl, i
}
#endif
+/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
+ entry. If RECORD_P is true and the target supports named sections,
+ the location of the NOPs will be recorded in a special object section
+ called "__patchable_function_entries"
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #19 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:b680b9049737198d010e49cf434704c6a6ed2b3f commit r11-7968-gb680b9049737198d010e49cf434704c6a6ed2b3f Author: Jakub Jelinek Date: Sat Apr 3 10:03:15 2021 +0200 rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125] The SECTION_LINK_ORDER changes broke powerpc64-linux ELFv1. Seems that the assembler/linker relies on the symbol mentioned for the "awo" section to be in the same section as the symbols mentioned in the relocations in that section (i.e. labels for the patchable area in this case). That is the case for most targets, including powerpc-linux 32-bit or powerpc64 ELFv2 (that one has -fpatchable-function-entry* support broken for other reasons and it doesn't seem to be a regression). But it doesn't work on powerpc64-linux ELFv1. We emit: .section".opd","aw" .align 3 _Z3foov: .quad .L._Z3foov,.TOC.@tocbase,0 .previous .type _Z3foov, @function .L._Z3foov: .section __patchable_function_entries,"awo",@progbits,_Z3foov .align 3 .8byte .LPFE1 .section.text._Z3foov,"axG",@progbits,_Z3foov,comdat .LPFE1: nop .LFB0: .cfi_startproc and because _Z3foov is in the .opd section rather than the function text section, it doesn't work. I'm afraid I don't know what exactly should be done, whether e.g. it could use .section __patchable_function_entries,"awo",@progbits,.L._Z3foov instead, or whether the linker should be changed to handle it as is, or something else. But because we have a P1 regression that didn't see useful progress over the 4 months since it has been filed and we don't really have much time, below is an attempt to do a targetted reversion of H.J's patch, basically act as if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1, but for 32-bit or 64-bit ELFv2 keep working as is. This would give us time to resolve it for GCC 12 properly. 2021-04-03 Jakub Jelinek PR testsuite/98125 * targhooks.h (default_print_patchable_function_entry_1): Declare. * targhooks.c (default_print_patchable_function_entry_1): New function, copied from default_print_patchable_function_entry with an added flags argument. (default_print_patchable_function_entry): Rewritten into a small wrapper around default_print_patchable_function_entry_1. * config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): Redefine. (rs6000_print_patchable_function_entry): New function. * g++.dg/pr93195a.C: Skip on powerpc*-*-* 64-bit.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 Jakub Jelinek changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #20 from Jakub Jelinek --- Regression is now fixed. Clones filed for GCC 12.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 Richard Biener changed: What|Removed |Added Summary|New test case |[11 Regression] New test |g++.dg/pr93195a.C in|case g++.dg/pr93195a.C in |r11-5656 has excess errors |r11-5656 has excess errors Target Milestone|--- |11.0 --- Comment #6 from Richard Biener --- dg-skip-if I suppose
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #7 from Alan Modra --- (In reply to Alan Modra from comment #5) > So the "o" flag symbol is one in the .opd section, rather than what would be > correct here, .L._Z3foov. Actually, that breakage happened recently with commit 694d4a6d0c4. ie. it looks like powerpc64 ELFv1 was broken by HJ's patch. I tend to care more about powerpc64 ELFv2, ie powerpc64le, which was broken prior to that patch.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #8 from Alan Modra --- Created attachment 49701 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49701&action=edit fix powerpc64 -fpatchable-function-entry This makes -fpatchable-function-entry do something sensible on powerpc64 ELFv1 and ELFv2. The ELFv2 fix is all rs6000 backend trickery, ELFv1 needs a change to generic code to use the function code section symbol rather than the function symbol as the "o" argument for the __patchable_function_entries .section directive. Regression testing only done powerpc64le-linux so far.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #9 from Jakub Jelinek --- What is the current status of this PR?
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #10 from seurer at gcc dot gnu.org --- It is still failing for me so I'd guess that Alan's patch is not submitted yet.
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 Alan Modra changed: What|Removed |Added Attachment #49701|0 |1 is obsolete|| --- Comment #11 from Alan Modra --- Created attachment 50038 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50038&action=edit ELFv2 support
[Bug testsuite/98125] [11 Regression] New test case g++.dg/pr93195a.C in r11-5656 has excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98125 --- Comment #12 from Alan Modra --- Created attachment 50039 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50039&action=edit ELFv1 support Revised patches. I wasn't happy with the use of a ".text" symbol in the previous patch since some targets don't call their text section ".text". These are really stage1 material. Usable -fpatchable-funtion-entry on powerpc64 is a new feature. I'm inclined to think the new test should just be skipped on powerpc64*-*. Bill, could you handle the testsuite fixes please?
