[PATCH] D52990: [MinGW] Allow using ubsan

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 8 14:48:32 PDT 2018


rnk added inline comments.


================
Comment at: lib/Driver/ToolChains/MinGW.cpp:266
+          // directives in the object files, but the static library needs
+          // -lpsapi unless the sanitizer was built targeting >= win7.
+          CmdArgs.push_back("-lpsapi");
----------------
mstorsjo wrote:
> smeenai wrote:
> > Isn't Windows 7 our minimum supported Windows version anyway? I can't find the documentation pointing to that, but I thought that was the policy. In particular, we require VS 2015 or above, which doesn't support anything older than Server 2008 anyway (including Vista and XP), and I doubt we'd have anyone using Server 2008.
> For llvm itself, yes, but this is for the runtimes. Or does the policy cover that as well?
> 
> Mingw upstream still(!) default to supporting xp onwards, while I'm configuring my own setups to default to vista. I guess making that 7 wouldn't be too much of an issue though.
> 
> FWIW, I included a corresponding change for ASAN here: https://reviews.llvm.org/rCRT343074, in adding a `append_list_if(MINGW psapi ASAN_DYNAMIC_LIBS)` in compiler-rt/trunk/lib/asan/CMakeLists.txt.
I think most of our policies have to do with LLVM's own build requirements. Clang has supported targeting older OSs for a while, and we shouldn't intentionally break it, at least. We probably don't want to support running the sanitizers on old Windows OSs, since they tend to want to use modern OS APIs. For example, I used the slim reader writer lock APIs in ASan to fix a race.

In any case, do you think it would be nicer to put this directive in the ubsan object files that use psapi? It seems lame for clang to have to know about what libraries the sanitizers use.


Repository:
  rC Clang

https://reviews.llvm.org/D52990





More information about the cfe-commits mailing list