Win64 lldb build broken, llvm::call_once and _mm_mfence

Subprojects LLDB
#1

The Win64 lldb build seems broken (at 294367).

I ran into this when trying to build the weekly snapshot
(http://www.llvm.org/builds/) which includes LLDB these days.

I suspect this might be related to Kamil's changes a few days ago. I
see Pavel committed something to fix Darwin afterwards.

Zach, do you know what's going on here? Do we have any buildbot
coverage for this?

   Creating library lib\liblldb.lib and object lib\liblldb.exp
lldbHost.lib(HostInfoWindows.cpp.obj) : error LNK2019: unresolved external symbo
l "void __cdecl llvm::sys::_mm_mfence(void)" (?_mm_mfence@sys@llvm@@YAXXZ) refer
enced in function "void __cdecl llvm::call_once<class <lambda_e212a11f7f891e804e
713e15728a6adc> >(struct llvm::once_flag &,class <lambda_e212a11f7f891e804e713e1
5728a6adc> &&)" (??$call_once@V<lambda_e212a11f7f891e804e713e15728a6adc>@@$$V@ll
vm@@YAXAEAUonce_flag@0@$$QEAV<lambda_e212a11f7f891e804e713e15728a6adc>@@@Z)
bin\liblldb.dll : fatal error LNK1120: 1 unresolved externals
LINK failed. with 1120

#2

Windows.h appears to #define MemoryFence _mm_fence. We should probably undef MemoryFence in llvm/Support/Atomic.h.

#3

Why doesn’t llvm::call_once() just use std::call_once on Windows?

#4

It’s a sad story. Read the comments and the review threads. It’s hilarious.

#5

It looks fine:

diff --git a/source/Host/windows/HostInfoWindows.cpp
b/source/Host/windows/HostInfoWindows.cpp
index a965ec0ea..5b38e6021 100644
--- a/source/Host/windows/HostInfoWindows.cpp
+++ b/source/Host/windows/HostInfoWindows.cpp
@@ -18,6 +18,7 @@
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Threading.h"
#include "llvm/Support/raw_ostream.h"

using namespace lldb_private;
@@ -90,8 +91,8 @@ bool HostInfoWindows::GetHostname(std::string &s) {
}

FileSpec HostInfoWindows::GetProgramFileSpec() {
- static std::once_flag g_once_flag;
- std::call_once(g_once_flag, []() {
+ static llvm::once_flag g_once_flag;
+ llvm::call_once(g_once_flag, []() {
     std::vector<wchar_t> buffer(PATH_MAX);
     ::GetModuleFileNameW(NULL, buffer.data(), buffer.size());
     std::string path;

and this looks suspicious

// std::call_once from libc++ is used on all Unix platforms. Other
// implementations like libstdc++ are known to have problems on NetBSD,
// OpenBSD and PowerPC.
#if defined(LLVM_ON_UNIX) && (defined(_LIBCPP_VERSION) ||
      \
    !(defined(__NetBSD__) || defined(__OpenBSD__) || defined(__ppc__)))
#define LLVM_THREADING_USE_STD_CALL_ONCE 1
#else
#define LLVM_THREADING_USE_STD_CALL_ONCE 0
#endif

This check defined(LLVM_ON_UNIX) looks wrong it assumes that Windows
needs call_once walkaround.

#6

Ah not, it's OK.

If the file is windows-only it can be switched back to std:: version.

#7

Is this the right review? https://reviews.llvm.org/D5922

Being that this was over 2 years ago, I suspect it was when we were supporting MSVC2012 and 2013. Now that we’re requiring MSVC2015, is it time to reconsider? I don’t see any links to an MS Connect issue, so I don’t know what the original bug was to know if it has been fixed.

#8

My fix was just correcting places Kamil forgot to update (or updated
over-excessively). It does not touch the root problem.

That said, our windows bot
<http://lab.llvm.org:8011/builders/lldb-windows7-android> is building
lldb with vs2015. It does not seem to have a problem with this.

#9

I only ran into problems on Win64. I guess your bot is doing a 32-bit build.

#10

Thank you for fixing it.

It was forgotten - I wanted to use consistently llvm:: namespace for
these functions across the LLDB codebase, especially since duplicated
code can be refactored to be shared and it would silently break in the
std:: version.

#11

I think we can. MSVC’s std::once_flag default constructor is constexpr now. It still generates dynamic initialization code if you use it as a static local, but MSVC defaults to using thread safe static initialization, so that isn’t a problem unless you disable it, which we don’t. We disable it in compiler-rt (/Zc:threadSafeInit-), but that doesn’t use this code.

#12

Should we do that now, as a way to fix this issue, or should we try to get another fix in first so we have more time to think about using std call_once?

#13

I'm happy as long as it builds :slight_smile:

#14

These issues should be resolved by r295013 and r295014.