From 17bab484aea43cf1a5247c823e036dfb52f5d92b Mon Sep 17 00:00:00 2001 From: Aliaksey Kandratsenka Date: Sat, 6 Feb 2021 13:21:12 -0800 Subject: [PATCH] always respect --enable-frame-pointers Previously it only was respected on x86_64, but this days lots of modern ABIs are without frame pointers by default (e.g. arm64 and riscv, and even older mips). --- CMakeLists.txt | 73 ++++++++++++---------------------- Makefile.am | 15 +++---- cmake/CheckNoFPByDefault.cmake | 41 ------------------- configure.ac | 23 +++-------- 4 files changed, 40 insertions(+), 112 deletions(-) delete mode 100644 cmake/CheckNoFPByDefault.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 7fa1d44b0..6dfe9a929 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,7 +58,6 @@ include(CPack) include(GNUInstallDirs) list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake) -include(CheckNoFPByDefault) include(DefineTargetVariables) include(FindObjcopyWithWeaken) include(PCFromUContext) @@ -263,34 +262,11 @@ if(gperftools_enable_libunwind) endif() endif() -# On x86_64, instead of libunwind, we can choose to compile with frame-pointers. -# Some x86_64 systems do not insert frame pointers by default. -# We want to see if the current system is one of those. +# On x86_64, we know that default is to omit frame pointer. if(x86_64) - check_omit_fp_by_default(omit_fp_by_default) + set(omit_fp_by_default ON) endif() -if(x86_64 AND omit_fp_by_default) - set(fp_option_enabled "Success") -else() - set(fp_option_enabled "Failed") - if(NOT x86_64) - set(fp_option_enabled "${fp_option_enabled}, not x86_64") - elseif(NOT omit_fp_by_default) - set(fp_option_enabled - "${fp_option_enabled}, frame pointer not omitted by default") - endif() - message(STATUS - "Enable option gperftools_enable_frame_pointers - ${fp_option_enabled}") -endif() - -cmake_dependent_option( - gperftools_enable_frame_pointers - "On x86_64 systems, compile with -fno-omit-frame-pointer (see INSTALL)" - OFF - "x86_64;omit_fp_by_default" - OFF) - # See if the compiler supports -Wno-unused-result. # Newer ubuntu's turn on -D_FORTIFY_SOURCE=2, enabling # __attribute__((warn_unused_result)) for things like write(), @@ -482,21 +458,23 @@ if(GPERFTOOLS_BUILD_CPU_PROFILER OR set(WITH_STACK_TRACE ON) endif() -if(gperftools_enable_frame_pointers AND - NOT unwind_libs AND - NOT gperftools_build_minimal) +# The following matters only if we're not using libunwind and if we +# care about backtrace capturing, and frame pointers are not available +# to capture backtraces. The idea is to warn user about less stable or +# known bad configurations (e.g. encourage to install libunwind). +if (NOT unwind_libs AND NOT gperftools_build_minimal AND + omit_fp_by_default AND NOT gperftools_enable_frame_pointers) if(HAVE_UNWIND_BACKTRACE) message(WARNING "No frame pointers and no libunwind. " "Using experimental backtrace capturing via libgcc. " "Expect crashy cpu profiler.") - if(gperftools_enable_stacktrace_via_backtrace) - message(WARNING "No frame pointers and no libunwind. " - "Using experimental backtrace(). " - "Expect crashy cpu profiler.") - else() - message(FATAL_ERROR "No frame pointers and no libunwind. " - "The compilation will fail.") - endif() + elseif(gperftools_enable_stacktrace_via_backtrace) + message(WARNING "No frame pointers and no libunwind. " + "Using experimental backtrace(). " + "Expect crashy cpu profiler.") + else() + message(FATAL_ERROR "No frame pointers and no libunwind. " + "The compilation will fail.") endif() endif() @@ -540,16 +518,17 @@ endif() # LIBSTDCXX_LA_LINKER_FLAG is used to fix a Solaris bug. add_link_options(${libstdcxx_la_linker_flag}) -# These are x86-specific, having to do with frame-pointers. In -# particular, some x86_64 systems do not insert frame pointers by -# default (all i386 systems that I know of, do. I don't know about -# non-x86 chips). We need to tell perftools what to do about that. -if(x86_64 AND x86_no_fp_by_default) - if(gperftools_enable_frame_pointers) - add_compile_options(-fno-omit-frame-pointer) - elseif(CMAKE_CXX_FLAGS MATCHES "-fomit-frame-pointer") - add_compile_definitions(NO_FRAME_POINTER) - endif() +option( + gperftools_enable_frame_pointers + "Compile with -fno-omit-frame-pointer (see INSTALL)" + OFF) + +if(gperftools_enable_frame_pointers) + add_compile_options(-fno-omit-frame-pointer) +endif() + +if(omit_fp_by_default AND NOT gperftools_enable_frame_pointers) + add_compile_definitions(NO_FRAME_POINTER) endif() # For windows systems (at least, mingw), we need to tell all our diff --git a/Makefile.am b/Makefile.am index bb689a59e..730582ece 100644 --- a/Makefile.am +++ b/Makefile.am @@ -51,15 +51,16 @@ AM_LDFLAGS = -no-undefined $(LIBSTDCXX_LA_LINKER_FLAG) # particular, some x86_64 systems do not insert frame pointers by # default (all i386 systems that I know of, do. I don't know about # non-x86 chips). We need to tell perftools what to do about that. -if X86_64_AND_NO_FP_BY_DEFAULT -if ENABLE_FRAME_POINTERS -AM_CXXFLAGS += -fno-omit-frame-pointer -else - # TODO(csilvers): check if -fomit-frame-pointer might be in $(CXXFLAGS), - # before setting this. +if OMIT_FP_BY_DEFAULT +if !ENABLE_FRAME_POINTERS AM_CXXFLAGS += -DNO_FRAME_POINTER endif !ENABLE_FRAME_POINTERS -endif X86_64_AND_NO_FP_BY_DEFAULT +endif OMIT_FP_BY_DEFAULT + +# respect --enable-frame-pointers regardless of architecture +if ENABLE_FRAME_POINTERS +AM_CXXFLAGS += -fno-omit-frame-pointer +endif ENABLE_FRAME_POINTERS # For windows systems (at least, mingw), we need to tell all our # tests to link in libtcmalloc using -u. This is because libtcmalloc diff --git a/cmake/CheckNoFPByDefault.cmake b/cmake/CheckNoFPByDefault.cmake deleted file mode 100644 index 0f32a47a6..000000000 --- a/cmake/CheckNoFPByDefault.cmake +++ /dev/null @@ -1,41 +0,0 @@ -function(check_omit_fp_by_default result) - set(src "${CMAKE_CURRENT_BINARY_DIR}/fp.c") - set(asm "${CMAKE_CURRENT_BINARY_DIR}/fp.s") - file(WRITE ${src} - "int f(int x) { return x; } int main() { return f(0); }") - execute_process( - COMMAND ${CMAKE_CXX_COMPILER} -O2 -S -o ${asm} ${src} - RESULT_VARIABLE compiled) - if(compiled EQUAL 0 AND EXISTS ${asm}) - file(STRINGS ${asm} asm_instructions) - set(fp_instructions "mov" "rsp" "rbp") - foreach(asm_instruction IN LISTS asm_instructions) - list(GET fp_instructions 0 fp_instruction) - if(asm_instruction MATCHES "${fp_instruction}") - list(REMOVE_AT fp_instructions 0) - endif() - - list(LENGTH fp_instructions len) - if(len EQUAL 0) - set(matched_all ON) - break() - endif() - endforeach() - - file(REMOVE ${asm}) - endif() - - file(REMOVE ${src}) - - if(NOT matched_all) - set(${result} ON PARENT_SCOPE) - endif() - - if(result) - set(fp_omitted "Success") - else() - set(fp_omitted "Failed") - endif() - message( - STATUS "Checking if frame pointers are omitted by default - ${fp_omitted}") -endfunction() diff --git a/configure.ac b/configure.ac index f1212b3d4..11eeb4d4e 100644 --- a/configure.ac +++ b/configure.ac @@ -286,24 +286,13 @@ AC_ARG_ENABLE(frame_pointers, AM_CONDITIONAL(ENABLE_FRAME_POINTERS, test "$enable_frame_pointers" = yes) AC_MSG_CHECKING([for x86 without frame pointers]) -# Some x86_64 systems do not insert frame pointers by default. -# We want to see if the current system is one of those. AC_COMPILE_IFELSE([AC_LANG_PROGRAM(, [return __x86_64__ == 1 ? 0 : 1])], [is_x86_64=yes], [is_x86_64=no]) -OLD_CFLAGS="$CFLAGS" -CFLAGS="$CFLAGS -S -O2 -o fp.s" -# This test will always fail because we don't name our output file properly. -# We do our own determination of success/failure in the grep, below. -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([int f(int x) {return x;}], [return f(0);])], - [:], [:]) -x86_no_fp_by_default=no -AS_IF([test "$is_x86_64" = yes && ! grep 'mov.*rsp.*rbp' fp.s >/dev/null 2>&1], [x86_no_fp_by_default=yes]) -AM_CONDITIONAL(X86_64_AND_NO_FP_BY_DEFAULT, - test "$x86_no_fp_by_default" = yes) -rm fp.s -CFLAGS="$OLD_CFLAGS" -AC_MSG_RESULT([$x86_no_fp_by_default]) - +omit_fp_by_default=no +AS_IF([test "$is_x86_64" = yes], [omit_fp_by_default=yes]) +AM_CONDITIONAL(OMIT_FP_BY_DEFAULT, + test "$omit_fp_by_default" = yes) +AC_MSG_RESULT([$omit_fp_by_default]) # We need to know if we're i386 so we can turn on -mmms, which is not # on by default for i386 (it is for x86_64). @@ -654,7 +643,7 @@ AC_CONFIG_FILES([Makefile src/gperftools/tcmalloc.h src/windows/gperftools/tcmalloc.h]) AC_OUTPUT -AS_IF([test "$x86_no_fp_by_default" = yes && test "x$enable_frame_pointers" != xyes && test "x$UNWIND_LIBS" = x && test "x$enable_minimal" != xyes], +AS_IF([test "$omit_fp_by_default" = yes && test "x$enable_frame_pointers" != xyes && test "x$UNWIND_LIBS" = x && test "x$enable_minimal" != xyes], [AS_IF([test "x$perftools_cv_have_unwind_backtrace" = xyes], [AC_MSG_WARN([No frame pointers and no libunwind. Using experimental backtrace capturing via libgcc. Expect crashy cpu profiler.])], [AS_IF([test "x$enable_backtrace" = xyes],