| Summary: | icf folds template functions with different exception handling semantics | ||
|---|---|---|---|
| Product: | binutils | Reporter: | gandhi.shaheen |
| Component: | gold | Assignee: | Cary Coutant <ccoutant> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ian, markus, oremanj |
| Priority: | P2 | ||
| Version: | 2.24 | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | 2017-01-24 00:00:00 | |
| Attachments: |
example source code
Proposed fix |
||
|
Description
gandhi.shaheen
2017-01-18 23:20:07 UTC
The problem here is that the two functions are identical in every aspect, except for the LSDA in .gcc_except_table. Since the LSDA has relocations pointing to the code, but not the other way around, the linker's ICF code doesn't spot the difference. It looks like we'll have to disable ICF for any functions that have an LSDA associated with them. A more ambitious approach might be to try to parse the LSDAs for each function and include their contents and relocations as part of the function's signature. *** Bug 22575 has been marked as a duplicate of this bug. *** *** Bug 23482 has been marked as a duplicate of this bug. *** As I pointed out in PR 23482, hot/cold function splitting (the new gcc -freorder-blocks-and-partition optimization) can make this an issue even without any LSDA at all. Seems a full fix will need to consider the CIE/FDE contents that are relevant to the function as well. Example uses asm for clarity, but I can obtain analogous issues using gcc-produced code, as demonstrated on the above PR. $ g++ -o t.nofold t.cc -fuse-ld=gold $ g++ -o t.fold t.cc -fuse-ld=gold -Wl,--icf=safe $ ./t.nofold caught 1: 42 caught 2: 42 $ ./t.fold caught 1: 42 terminate called after throwing an instance of 'int' Aborted (core dumped) $ cat t.cc extern "C" int printf(const char *fmt, ...); void throw_it() { throw 42; } extern void call_1(void(*)()); extern void call_2(void(*)()); asm(R"( .section .text._Z6call_1PFvvE,"ax",@progbits .globl _Z6call_1PFvvE .type _Z6call_1PFvvE, @function _Z6call_1PFvvE: .cfi_startproc pushq %rbx .cfi_def_cfa_offset 16 .cfi_offset 3, -16 jmp _Z6call_1PFvvE.cold .cfi_endproc .section .text.unlikely._Z6call_1PFvvE,"ax",@progbits .type _Z6call_1PFvvE.cold, @function _Z6call_1PFvvE.cold: .cfi_startproc .cfi_def_cfa_offset 16 .cfi_offset 3, -16 call *%rdi .cfi_endproc .section .text._Z6call_2PFvvE,"ax",@progbits .globl _Z6call_2PFvvE .type _Z6call_2PFvvE, @function _Z6call_2PFvvE: .cfi_startproc subq $24, %rsp .cfi_def_cfa_offset 32 jmp _Z6call_2PFvvE.cold .cfi_endproc .section .text.unlikely._Z6call_2PFvvE,"ax",@progbits .type _Z6call_2PFvvE.cold, @function _Z6call_2PFvvE.cold: .cfi_startproc .cfi_def_cfa_offset 32 call *%rdi .cfi_endproc )"); int main() { try { call_1(throw_it); } catch (int val) { printf("caught 1: %d\n", val); } try { call_2(throw_it); } catch (int val) { printf("caught 2: %d\n", val); } } Created attachment 11278 [details] Proposed fix I believe this patch fixes all the issues described in this thread. I also submitted it to the mailing list: https://sourceware.org/ml/binutils/2018-09/msg00346.html The master branch has been updated by Cary Coutant <ccoutant@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e173ea00c2941af42ea4e2267446d6039a70da6e commit e173ea00c2941af42ea4e2267446d6039a70da6e Author: Joshua Oreman <oremanj@hudson-trading.com> Date: Sat May 11 07:27:10 2019 +0800 Fix problem with ICF where diffs in EH frame info is ignored. PR gold/21066 * gc.h (gc_process_relocs): Track relocations in .eh_frame sections when ICF is enabled, even though the .eh_frame sections themselves are not foldable. * icf.cc (get_section_contents): Change arguments to permit operation on just part of a section. Include extra identity regions in the referring section's contents recursively. (match_sections): Lock object here instead of in get_section_contents so that get_section_contents can operate recursively. (Icf::add_ehframe_links): New method. (Icf::find_identical_sections): Pass .eh_frame sections to add_ehframe_links(). Increase default iteration count from 2 to 3 because handling exception info typically requires one extra iteration. * icf.h (Icf::extra_identity_list_): New data member with accessor. (is_section_foldable_candidate): Include .gcc_except_table sections. * options.h: Update documentation for new default ICF iteration count. * testsuite/Makefile.am (icf_test_pr21066): New test case. * testsuite/Makefile.in: Regenerate. * testsuite/icf_test_pr21066.cc: New source file. * testsuite/icf_test_pr21066.sh: New test script. Fixed on master. |