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).
sleepwalker2017 · Feb 15, 2021 · 17bab48 · 17bab48
1 parent 22c0ece
commit 17bab48
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 112 deletions.
diff --git 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
@@ -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
diff --git 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],