Re: [patch] Fix PR target/81361
On Mon, Sep 18, 2017 at 09:50:45AM +0200, Eric Botcazou wrote: > .set L$set$24,LEFDE3-LASFDE3 > .long L$set$24 # FDE Length > LASFDE3: > .long LASFDE3-EH_frame1 # FDE CIE offset > .quad LCOLDB1-. # FDE initial location > .set L$set$25,LCOLDE1-LCOLDB1 > .quad L$set$25 # FDE address range > .byte 0x8 # uleb128 0x8; Augmentation size > .quad LLSDAC5-. # Language Specific Data Area > .byte 0x1 # DW_CFA_set_loc > .quad LCFI1-. > .byte 0xe # DW_CFA_def_cfa_offset > .byte 0x10# uleb128 0x10 > .byte 0x83# DW_CFA_offset, column 0x3 > .byte 0x2 # uleb128 0x2 > > Note the DW_CFA_set_loc operation: it's the only case where the compiler > emits > it (DW_CFA_advance_loc4 is usually emitted) and is the source of the problem, > since it appears that the PC-relative relocation is not applied to the > operand > of the DW_CFA_set_loc (unlike to the 2 other cases in the FDE). That sounds like a Darwin bug in handling of DW_CFA_set_loc in .eh_frame section, the encoding/size of the DW_CFA_set_loc operand is an encoded pointer, always absolute address in .debug_frame section, and whatever the CIE augmentation says should be used otherwise, i.e. the same as e.g. FDE initial location pointer. As that is LCOLDB1-. in the same FDE, it means LCFI1-. is right. That said, there is indeed no reason to emit DW_CFA_set_loc when we have a label, so your patch is ok for trunk. That doesn't mean Darwin shouldn't be fixed. libgcc unwind-dw2.c for DW_CFA_set_loc uses read_encoded_value and so I believe should DTRT. > This DW_CFA_set_loc instruction is emitted by add_cfis_to_fde for the second > FDE generated for the cold part of a function but doesn't seem necessary any > more, since there is a label (LCOLDB1) to be used now (this can also be seen > on Linux with the -fno-dwarf2-cfi-asm option). > > Bootstrapped/regtested on x86-64/Linux by me and various versions of Darwin > by > Iain, Dominique and myself. OK for the mainline? > > > 2017-09-18 Eric Botcazou > > PR target/81361 > * dwarf2cfi.c (add_cfis_to_fde): Do not generate DW_CFA_set_loc > after switching to a new text section. Jakub
[patch] Fix PR target/81361
Hi, exception handling is currently broken in all languages for Darwin at -O2 on the mainline because of what appears to be a bug in either the assembler or the system unwinder. The problem occurs when the compiler decides to split a function into hot & cold parts and the cold part is active wrt EH; in this case, the compiler generates a FDE for each part (the Darwin port doesn't use the CFI assembler directives) and the second FDE looks like: .set L$set$24,LEFDE3-LASFDE3 .long L$set$24 # FDE Length LASFDE3: .long LASFDE3-EH_frame1 # FDE CIE offset .quad LCOLDB1-. # FDE initial location .set L$set$25,LCOLDE1-LCOLDB1 .quad L$set$25 # FDE address range .byte 0x8 # uleb128 0x8; Augmentation size .quad LLSDAC5-. # Language Specific Data Area .byte 0x1 # DW_CFA_set_loc .quad LCFI1-. .byte 0xe # DW_CFA_def_cfa_offset .byte 0x10# uleb128 0x10 .byte 0x83# DW_CFA_offset, column 0x3 .byte 0x2 # uleb128 0x2 Note the DW_CFA_set_loc operation: it's the only case where the compiler emits it (DW_CFA_advance_loc4 is usually emitted) and is the source of the problem, since it appears that the PC-relative relocation is not applied to the operand of the DW_CFA_set_loc (unlike to the 2 other cases in the FDE). This DW_CFA_set_loc instruction is emitted by add_cfis_to_fde for the second FDE generated for the cold part of a function but doesn't seem necessary any more, since there is a label (LCOLDB1) to be used now (this can also be seen on Linux with the -fno-dwarf2-cfi-asm option). Bootstrapped/regtested on x86-64/Linux by me and various versions of Darwin by Iain, Dominique and myself. OK for the mainline? 2017-09-18 Eric Botcazou PR target/81361 * dwarf2cfi.c (add_cfis_to_fde): Do not generate DW_CFA_set_loc after switching to a new text section. -- Eric BotcazouIndex: dwarf2cfi.c === --- dwarf2cfi.c (revision 252749) +++ dwarf2cfi.c (working copy) @@ -2209,20 +2209,13 @@ add_cfis_to_fde (void) { dw_fde_ref fde = cfun->fde; rtx_insn *insn, *next; - /* We always start with a function_begin label. */ - bool first = false; for (insn = get_insns (); insn; insn = next) { next = NEXT_INSN (insn); if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS) - { - fde->dw_fde_switch_cfi_index = vec_safe_length (fde->dw_fde_cfi); - /* Don't attempt to advance_loc4 between labels - in different sections. */ - first = true; - } + fde->dw_fde_switch_cfi_index = vec_safe_length (fde->dw_fde_cfi); if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_CFI) { @@ -2247,8 +2240,7 @@ add_cfis_to_fde (void) /* Set the location counter to the new label. */ xcfi = new_cfi (); - xcfi->dw_cfi_opc = (first ? DW_CFA_set_loc - : DW_CFA_advance_loc4); + xcfi->dw_cfi_opc = DW_CFA_advance_loc4; xcfi->dw_cfi_oprnd1.dw_cfi_addr = label; vec_safe_push (fde->dw_fde_cfi, xcfi); @@ -2263,7 +2255,6 @@ add_cfis_to_fde (void) insn = NEXT_INSN (insn); } while (insn != next); - first = false; } } }