[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

30

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

30

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

30

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

[mstorsjo]

libunwind/src/libunwind.cpp

30

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/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 dependent on C++ library which means that
+// we need our own definition of inline placement new that's used in this file.
+inline void* operator new(size_t, void* __p) { return __p; }
+
 /// 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 *);