This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix P81033 for FDEs in partitioned code.
On Tue, Aug 21, 2018 at 11:45 AM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
>
> > On 20 Aug 2018, at 08:27, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Aug 14, 2018 at 10:18 PM Mike Stump <mikestump@comcast.net> wrote:
> >>
> >> On Aug 14, 2018, at 4:20 AM, Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>> When function sub-sections are enabled, Darwin’s assembler needs the FDE local start
> >>> label for each sub-section to follow a linker-visible one so that the FDE will be correctly
> >>> associated with the code of the subsection.
> >>>
> >>> The current code in final.c emits a linker-visible symbol, as needed by several targets.
> >>> However the local label used to define the FDE start precedes the linker-visible one
> >>> which, for Darwin causes it (the FDE start) to be associated with the previous linker-
> >>> visible symbol (or the section start if there isn’t one). This applies regardless of the
> >>> actual address of the label, for toolchain assemblers that have strict interpretation of
> >>> the Darwin sub-sections-via-symbols ABI.
> >>>
> >>> The patch adds a new local label (analogous to the "LFBn” emitted for the regular
> >>> function starts) just after the linker-visible label emitted after switching text section.
> >>> The FDE second entry is made to point to this instead of the LcoldStartn one. This
> >>> should be a no-op for targets using .cfi_ and for targets without sub-sections-via-symbols.
> >>>
> >>> Bootstrapped on x86_64 and i686 linux and on a number of Darwin platforms (from
> >>> i686-darwin9 to x86_64-darwin17).
> >>>
> >>> OK for trunk?
> >>> open branches? (although it's a regression on 8, it’s a latent wrong-code on all branches)
> >>
> >> I'm fine with the darwin aspects of it, but I think it needs review/approval by final.c/dwarf people.
> >
> > The approach looks fine (though we have extra labels even for targets
> > that do not need it). But,
> >
> > + fde->dw_fde_second_begin = xstrdup (label);
> >
> > to me it looks like dw_fde_node is GC allocated so the above needs ggc_strdup.
>
> Revised below, (bootstrapped Linux and Darwin).
>
> OK to apply now?
OK.
Richard.
> thanks
> Iain
>
> gcc/
>
> PR bootstrap/81033
> PR target/81733
> PR target/52795
> * gcc/dwarf2out.c (FUNC_SECOND_SECT_LABEL): New.
> (dwarf2out_switch_text_section): Generate a local label for the second
> function sub-section and apply it as the second FDE start label.
> * gcc/final.c (final_scan_insn_1): Emit second FDE label after the second
> sub-section start.
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 236f199..6a66de7 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -297,6 +297,10 @@ static unsigned int rnglist_idx;
> #define FUNC_BEGIN_LABEL "LFB"
> #endif
>
> +#ifndef FUNC_SECOND_SECT_LABEL
> +#define FUNC_SECOND_SECT_LABEL "LFSB"
> +#endif
> +
> #ifndef FUNC_END_LABEL
> #define FUNC_END_LABEL "LFE"
> #endif
> @@ -1212,21 +1216,24 @@ static void set_cur_line_info_table (section *);
> void
> dwarf2out_switch_text_section (void)
> {
> + char label[MAX_ARTIFICIAL_LABEL_BYTES];
> section *sect;
> dw_fde_ref fde = cfun->fde;
>
> gcc_assert (cfun && fde && fde->dw_fde_second_begin == NULL);
>
> + ASM_GENERATE_INTERNAL_LABEL (label, FUNC_SECOND_SECT_LABEL,
> + current_function_funcdef_no);
> +
> + fde->dw_fde_second_begin = ggc_strdup (label);
> if (!in_cold_section_p)
> {
> fde->dw_fde_end = crtl->subsections.cold_section_end_label;
> - fde->dw_fde_second_begin = crtl->subsections.hot_section_label;
> fde->dw_fde_second_end = crtl->subsections.hot_section_end_label;
> }
> else
> {
> fde->dw_fde_end = crtl->subsections.hot_section_end_label;
> - fde->dw_fde_second_begin = crtl->subsections.cold_section_label;
> fde->dw_fde_second_end = crtl->subsections.cold_section_end_label;
> }
> have_multiple_function_sections = true;
> diff --git a/gcc/final.c b/gcc/final.c
> index 842e5e0..6943c07 100644
> --- a/gcc/final.c
> +++ b/gcc/final.c
> @@ -2232,6 +2232,9 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
> ASM_OUTPUT_LABEL (asm_out_file,
> IDENTIFIER_POINTER (cold_function_name));
> #endif
> + if (dwarf2out_do_frame ()
> + && cfun->fde->dw_fde_second_begin != NULL)
> + ASM_OUTPUT_LABEL (asm_out_file, cfun->fde->dw_fde_second_begin);
> }
> break;
>
>