[libunwind] Drop the dependency on <algorithm>, add placement new inline

[phosek]

Summary

We haven't eliminated C++ library dependency altogether in D57251,
UnwindCursor.hpp had an unused dependency on <algorithm> which was
pulling in other C++ headers. Removing that dependency also revealed
(correctly) that we need our own global placement new declaration. Now
libunwind should be independent of the C++ library.

[phosek]

Comment added.

[mcgrathr]

Should you add -nostdinc++ to the compilation flags to drive home that this code can't depend on any library headers?

[ldionne]

libunwind/src/libunwind.cpp

26

Why do we need to define this? I thought placement new was handled by Clang directly, without requiring a declaration?

[phosek]

libunwind/src/libunwind.cpp

26

That's what I assumed but that doesn't seem to be the case, without this definition the compilation fails because Clang cannot find the operator new.

[phosek]

In D57262#1372004, @mcgrathr wrote:

Should you add -nostdinc++ to the compilation flags to drive home that this code can't depend on any library headers?

Done, we were already using -nostdinc++ but we were also adding libc++ include path explicitly into the list of include directories, so I dropped that logic and made -nostdinc++ unconditional.

[phosek]

libunwind/src/libunwind.cpp

26

Actually I think you're right, if I only provide declaration, Clang handles the definition.

[mstorsjo]

libunwind/src/libunwind.cpp

26

Spelling nitpick: "should not depend on", or "is not dependent on"

[mstorsjo]

This patch breaks compilation of Unwind-seh.cpp, which also needs a declaration of the placement new operator.

[phosek]

In D57262#1372385, @mstorsjo wrote:

This patch breaks compilation of Unwind-seh.cpp, which also needs a declaration of the placement new operator.

Thanks for checking, this should be fixed now.

[mstorsjo]

In D57262#1373225, @phosek wrote:

In D57262#1372385, @mstorsjo wrote:

This patch breaks compilation of Unwind-seh.cpp, which also needs a declaration of the placement new operator.

Thanks for checking, this should be fixed now.

Thanks, now it does indeed seem to build fine for me.

[ldionne]

libunwind/src/libunwind.cpp

15

Just to confirm -- this is handled by #include <stdlib.h> because we use the global ::getenv() function, right?

[phosek]

libunwind/src/libunwind.cpp

15

Yes, correct.

[phosek]

I missed some more C++ library uses in UnwindCursor.hpp and Unwind-EHABI.cpp which broke Arm bots and so I had to revert the change. This should now be addressed. Please let me know if this is okay to submit.

[mstorsjo]

In D57262#1374956, @phosek wrote:

I missed some more C++ library uses in UnwindCursor.hpp and Unwind-EHABI.cpp which broke Arm bots and so I had to revert the change. This should now be addressed. Please let me know if this is okay to submit.

I believe the code looks good, although I haven't tested the EHABI unwinding code after this change to see if it still works.

[ldionne]

As much as I hate duplicating stuff like upper_bound, I find the removal of the circular dependency so nice that I don't care.

[phosek]

In D57262#1375484, @ldionne wrote:

As much as I hate duplicating stuff like upper_bound, I find the removal of the circular dependency so nice that I don't care.

I have exactly the same feeling about this one ;)

libunwind/CMakeLists.txt

 # Exceptions
 # Catches C++ exceptions only and tells the compiler to assume that extern C
 # functions never throw a C++ exception.
 unwind_append_if(LIBUNWIND_CXX_FLAGS LIBUNWIND_HAS_FSTRICT_ALIASING_FLAG -fstrict-aliasing)
 unwind_append_if(LIBUNWIND_CXX_FLAGS LIBUNWIND_HAS_EHSC_FLAG -EHsc)
 
 unwind_append_if(LIBUNWIND_C_FLAGS LIBUNWIND_HAS_FUNWIND_TABLES -funwind-tables)
 
+# Ensure that we don't depend on C++ standard library.
+unwind_append_if(LIBUNWIND_CXX_FLAGS LIBUNWIND_HAS_NOSTDINCXX_FLAG -nostdinc++)
+
 # Assert
 string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 if (LIBUNWIND_ENABLE_ASSERTIONS)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   if (NOT MSVC)
     list(APPEND LIBUNWIND_COMPILE_FLAGS -D_DEBUG)
   endif()
 

 endif()
 
 #===============================================================================
 # Setup Source Code
 #===============================================================================
 
 include_directories(include)
 
-find_path(
-  LIBUNWIND_LIBCXX_INCLUDES_INTERNAL
-  __libcpp_version
-  PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxx/include
-        ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx/include
-        ${LLVM_MAIN_SRC_DIR}/../libcxx/include
-  NO_DEFAULT_PATH
-  NO_CMAKE_FIND_ROOT_PATH
-  )
-if ((NOT LIBUNWIND_STANDALONE_BUILD OR HAVE_LIBCXX) AND
-    IS_DIRECTORY "${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")
-  set(LIBUNWIND_CXX_INCLUDE_PATHS_DEFAULT "${LIBUNWIND_LIBCXX_INCLUDES_INTERNAL}")
-endif()
-
-set(LIBUNWIND_CXX_INCLUDE_PATHS "${LIBUNWIND_CXX_INCLUDE_PATHS_DEFAULT}" CACHE PATH
-    "Paths to C++ header directories separated by ';'.")
-
-if (NOT LIBUNWIND_CXX_INCLUDE_PATHS STREQUAL "")
-  list(APPEND LIBUNWIND_CXX_FLAGS -nostdinc++)
-  include_directories("${LIBUNWIND_CXX_INCLUDE_PATHS}")
-endif()
-
 add_subdirectory(src)
 
 if (LIBUNWIND_INCLUDE_DOCS)
   add_subdirectory(docs)
 endif()
 
 if (EXISTS ${LLVM_CMAKE_PATH})
   add_subdirectory(test)
 endif()

libunwind/src/Unwind-seh.cpp

 #define STATUS_GCC_THROW MAKE_GCC_EXCEPTION(0) // 0x20474343
 /// SEH exception raised by libunwind to initiate phase 2 of exception
 /// handling.
 #define STATUS_GCC_UNWIND MAKE_GCC_EXCEPTION(1) // 0x21474343
 
 /// Class of foreign exceptions based on unrecognized SEH exceptions.
 static const uint64_t kSEHExceptionClass = 0x434C4E4753454800; // CLNGSEH\0
 
+// libunwind does not and should not depend on C++ library which means that we
+// need our own declaration of global placement new.
+void *operator new(size_t, void*);
+
 /// Exception cleanup routine used by \c _GCC_specific_handler to
 /// free foreign exceptions.
 static void seh_exc_cleanup(_Unwind_Reason_Code urc, _Unwind_Exception *exc) {
   (void)urc;
   if (exc->exception_class != kSEHExceptionClass)
     _LIBUNWIND_ABORT("SEH cleanup called on non-SEH exception");
   free(exc);
 }

libunwind/src/UnwindCursor.hpp

 //===------------------------- UnwindCursor.hpp ---------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
 // C++ interface to lower levels of libunwind
 //===----------------------------------------------------------------------===//
 
 #ifndef __UNWINDCURSOR_HPP__
 #define __UNWINDCURSOR_HPP__
 
-#include <algorithm>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unwind.h>
 
 #ifdef _WIN32
   #include <windows.h>
   #include <ntverp.h>

libunwind/src/libunwind.cpp

 //===--------------------------- libunwind.cpp ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
 //  Implements unw_* functions from <libunwind.h>
 //
 //===----------------------------------------------------------------------===//
 
 #include <libunwind.h>
 
-#ifndef NDEBUG
-#include <cstdlib> // getenv
-#endif
-
 #include "libunwind_ext.h"
 #include "config.h"
 
 #include <stdlib.h>
 
 
 #if !defined(__USING_SJLJ_EXCEPTIONS__)
 #include "AddressSpace.hpp"
 #include "UnwindCursor.hpp"
 
 using namespace libunwind;
 
+// libunwind does not and should not depend on C++ library which means that we
+// need our own declaration of global placement new.
+void *operator new(size_t, void*);
+
 /// internal object to represent this processes address space
 LocalAddressSpace LocalAddressSpace::sThisAddressSpace;
 
 _LIBUNWIND_EXPORT unw_addr_space_t unw_local_addr_space =
     (unw_addr_space_t)&LocalAddressSpace::sThisAddressSpace;
 
 /// record the registers and stack position of the caller
 extern int unw_getcontext(unw_context_t *);