Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC PR109267: g++ -Og generates empty functions with incorrect FDE; the output doesn't handle exceptions when linked with ld.lld #61434

Open
FlorianChevassu opened this issue Mar 15, 2023 · 6 comments
Labels
invalid Resolved as invalid, i.e. not a bug lld:ELF lld

Comments

@FlorianChevassu
Copy link

Consider the following simple example:

lib.cpp

#include "lib.h"
#include <stdexcept>
void test(const T_value& entry_value) {
    boost::apply_visitor([](const auto& value) { test(value); },
                         entry_value);
}
void test(const Obj_1& ) {
    throw std::runtime_error("Obj_1");
}
void test(const Obj_2& ) {
    throw std::runtime_error("Obj_2");
}

lib.h

#pragma once

#include <string>
#include <boost/variant.hpp>

struct Obj_1 {
    std::string oid;
};
struct Obj_2 {
    std::string oid;
};

using T_value = boost::make_recursive_variant<
        Obj_1,
        Obj_2>::type;
void test(const T_value&);
void test(const Obj_2&);
void test(const Obj_1&);

main.cpp

#include "lib.h"
#include <stdexcept>

int main() {
    try {
        test(Obj_2{"foo"});
    } catch (const std::runtime_error& e) {
    }
    try {
        test(Obj_1{"foo"});
    } catch (const std::runtime_error& e) {
    }
} 

When building the code using g++ and linking with lld, the exception thrown by the call to test(Obj_1{"foo"}); is never caught, and the program terminates.

Please note that the program executes successfully with the following configurations:

  • compile with g++ and link with ld,
  • compile with clang++ and link with lld.
  • if we add a dummy function implementation at the top of lib.cpp

The versions of the software used are the following:

  • LLVM: 15.0.7 (installed with bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)")
  • GCC 12.2.0

A docker image with all the dependencies reproducing the issue is available in the example.

Please let me know if you need more information, or if the issue should be on the GCC side.

Best regards

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 21, 2023

@llvm/issue-subscribers-lld-elf

@smithp35
Copy link
Collaborator

I wanted to check whether this was related to https://reviews.llvm.org/D143136 which was a .eh_frame modification that isn't in LLVM 15 sadly it didn't make a difference.

The only observations I can make in the time I had available:
GNU ld map file has the lines:

discarding zero address range FDE in src/build/lib.o(.eh_frame).
discarding zero address range FDE in src/build/lib.o(.eh_frame).

LLD does not discard these and they remain in the consolidated .eh_frame.

Indeed lib.o when compiled by GCC does have .eh_frame entries with a couple of zero address range entries, but clang and gcc-9 (on my system) do not produce these entries. However adding a dummy function to lib.cpp doesn't make these zero address range entries disappear.

Looking at the binutils bugzilla after tracing the commit that added those messages brings up https://sourceware.org/bugzilla/show_bug.cgi?id=17447 which states that in some circumstances these empty address ranges can cause an unwinder problem.

So it looks like it may be either GCC is wrong to output these empty address ranges or LLD is wrong not to discard them. Given that GCC12 already exists it implies that LLD will need to be able to handle these.

I personally won't have any spare time to look at this, hopefully MaskRay will when he is back from break.

@MaskRay
Copy link
Member

MaskRay commented Mar 23, 2023

Below are the first few lines of lib.s when I compile lib.cpp with g++ -Og.
There are two empty functions and their FDE information has the same location as _Z4testRK5Obj_1's.
When GNU ld combines input .eh_frame sections, it appears to keep one FDE when multiple FDEs have the same location.
ld.lld doesn't do this. When throwing an exception, the FDE associated with an empty PC range is picked which is not considered a catch handler.
libstdc++ will call std::terminate since there is no catching handler.

I think g++ -Og should avoid generating .cfi_startproc/.cfi_endproc with such empty functions.
Filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109267

        .file   "lib.cpp"
        .text
.Ltext0:
        .file 0 "/tmp/c/example/src/build" "../lib.cpp"
        .type   _ZN5boost6detail7variant22visitation_impl_invokeINS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SC_EELb0EEEPKvNSA_18has_fallback_type_EEENSD_11result_typeEiRSD_T0_PNS1_22apply_visitor_unrolledET1_l, @function
_ZN5boost6detail7variant22visitation_impl_invokeINS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SC_EELb0EEEPKvNSA_18has_fallback_type_EEENSD_11result_typeEiRSD_T0_PNS1_22apply_visitor_unrolledET1_l:
.LFB3842:
        .file 1 "/usr/include/boost/variant/detail/visitation_impl.hpp"
        .loc 1 165 1 view -0
        .cfi_startproc
.LBB98:
.LBI98:
        .file 2 "/usr/include/boost/variant/detail/forced_return.hpp"
        .loc 2 35 1 view .LVU1
        .loc 2 35 1 is_stmt 0 view .LVU2
.LBE98:
        .cfi_endproc
.LFE3842:
        .size   _ZN5boost6detail7variant22visitation_impl_invokeINS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SC_EELb0EEEPKvNSA_18has_fallback_type_EEENSD_11result_typeEiRSD_T0_PNS1_22apply_visitor_unrolledET1_l, .-_ZN5boost6detail7variant22visitation_impl_invokeINS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SC_EELb0EEEPKvNSA_18has_fallback_type_EEENSD_11result_typeEiRSD_T0_PNS1_22apply_visitor_unrolledET1_l
        .type   _ZN5boost6detail7variant15visitation_implIN4mpl_4int_ILi20EEENS1_20visitation_impl_stepINS_3mpl6l_iterINS7_5l_endEEESA_EENS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SL_EELb0EEEPKvNSJ_18has_fallback_type_EEENT1_11result_typeEiiRSV_T2_NS3_5bool_ILb1EEET3_PSM_PT0_, @function
_ZN5boost6detail7variant15visitation_implIN4mpl_4int_ILi20EEENS1_20visitation_impl_stepINS_3mpl6l_iterINS7_5l_endEEESA_EENS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SL_EELb0EEEPKvNSJ_18has_fallback_type_EEENT1_11result_typeEiiRSV_T2_NS3_5bool_ILb1EEET3_PSM_PT0_:
.LFB3844:
        .loc 1 184 1 is_stmt 1 view -0
        .cfi_startproc
.LBB99:
.LBI99:
        .loc 2 35 1 view .LVU4
        .loc 2 35 1 is_stmt 0 view .LVU5
.LBE99:
        .cfi_endproc
.LFE3844:
        .size   _ZN5boost6detail7variant15visitation_implIN4mpl_4int_ILi20EEENS1_20visitation_impl_stepINS_3mpl6l_iterINS7_5l_endEEESA_EENS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SL_EELb0EEEPKvNSJ_18has_fallback_type_EEENT1_11result_typeEiiRSV_T2_NS3_5bool_ILb1EEET3_PSM_PT0_, .-_ZN5boost6detail7variant15visitation_implIN4mpl_4int_ILi20EEENS1_20visitation_impl_stepINS_3mpl6l_iterINS7_5l_endEEESA_EENS1_14invoke_visitorINS1_15result_wrapper1IZ4testRKNS_7variantINS1_14recursive_flagI5Obj_1EEJ5Obj_2EEEEUlRKT_E_SL_EELb0EEEPKvNSJ_18has_fallback_type_EEENT1_11result_typeEiiRSV_T2_NS3_5bool_ILb1EEET3_PSM_PT0_

@MaskRay MaskRay changed the title -fuse-ld=lld: Wrong code generation ? GCC PR109267: g++ -Og generates empty functions with incorrect FDE; the output doesn't handle exceptions when linked with ld.lld Mar 23, 2023
@MaskRay MaskRay added the invalid Resolved as invalid, i.e. not a bug label Mar 23, 2023
@apinski-cavium
Copy link

Note clang can produce the same bad assembly by:
void f(){__builtin_unreachable();}

So maybe it is not a GCC bug but rather a binutils one ...

@apinski-cavium
Copy link

My bet is you might be able to reproduce the same issue with clang by adding:

void g(void) {__builtin_unreachable();}
void h(void) {__builtin_unreachable();}

To the end of lib.cpp

@MaskRay
Copy link
Member

MaskRay commented Mar 23, 2023

You are right about the Clang behavior. So the problem can be addressed in any of the 4 places:

  • GCC and Clang can avoid generating .cfi_startproc/.cfi_endproc for empty functions (literally no instruction (no RET or NOP or TRAP), st_size is zero)
  • GNU assembler and LLVM integrated assembler can avoid generating FDE for .cfi_startproc/.cfi_endproc associated with empty functions.
  • Linkers can discard FDE associated with empty functions. I think GNU ld seems to do this, but I am unsure whether it's enforced. Even if I know that --icf=all and --gc-sections have some effects on .eh_frame linking, without using the two features discarding input sections magically appears to deviate from the ELF spirit.
  • libgcc_s/libunwind ignore FDE associated with empty functions.

Linker discarding FDE doesn't fulfill the ELF spirit to me: input sections should be combined with no magical dropping rules. So perhaps assemblers are the right place to stop generating these FDE.

zero9178 added a commit to JLLVM/JLLVM that referenced this issue Dec 28, 2023
The combination of exception handling, optimizations creating empty functions and using LLD exposes an upstream issue in the toolchain which leads to `libgcc` and `libunwind` crashing on malformed data: llvm/llvm-project#61434

The issue is most easily worked around by not using LLD in release builds.
Other fixes such as reducing our use of `llvm_unreachable` or checking whether there are compiler options to avoid emitting zero-sized functions can be considered in the future.

Until this issue is fixed upstream, change our CI to not use LLD in fully optimized builds and add code to cmake to warn when using release builds with LLD to avoid the issue.
zero9178 added a commit to JLLVM/JLLVM that referenced this issue Dec 28, 2023
The combination of exception handling, optimizations creating empty functions and using LLD exposes an upstream issue in the toolchain which leads to `libgcc` and `libunwind` crashing on malformed data: llvm/llvm-project#61434

The issue is most easily worked around by not using LLD in release builds.
Other fixes such as reducing our use of `llvm_unreachable` or checking whether there are compiler options to avoid emitting zero-sized functions can be considered in the future.

Until this issue is fixed upstream, change our CI to not use LLD in fully optimized builds and add code to cmake to warn when using release builds with LLD to avoid the issue.
github-merge-queue bot pushed a commit to JLLVM/JLLVM that referenced this issue Dec 28, 2023
The combination of exception handling, optimizations creating empty
functions and using LLD exposes an upstream issue in the toolchain which
leads to `libgcc` and `libunwind` crashing on malformed data:
llvm/llvm-project#61434

This has also lead to our CI crashing:
https://github.com/JLLVM/JLLVM/actions/runs/7346263904/job/20000726099

The issue is most easily worked around by not using LLD in release
builds. Other fixes such as reducing our use of `llvm_unreachable` or
checking whether there are compiler options to avoid emitting zero-sized
functions can be considered in the future.

Until this issue is fixed upstream, change our CI to not use LLD in
fully optimized builds and add code to cmake to warn when using release
builds with LLD to avoid the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Resolved as invalid, i.e. not a bug lld:ELF lld
Projects
None yet
Development

No branches or pull requests

7 participants