mstorsjo 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");
----------------
smeenai wrote:
> rnk wrote:
> > 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.
> Ah, I wasn't thinking of the LLVM/runtimes distinction. SRWLocks would
> require Vista and above though, and at that point just going for 7 and above
> would make sense.
>
> This is a bit orthogonal to the patch though, so sorry for side-tracking
> things. +1 for Reid's suggestion of embedding the psapi directive in object
> files.
> 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.
Yes, I've tried to keep things simple by requiring Vista in a number of other
places as well, especially for SRWLocks and other threading related things.
Since Vista is pretty much extinct, requiring 7 wouldn't be that big of a
change though. I have no intention of trying to support sanitizers on anything
older than that.
> 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.
It would definitely be nicer - I'll have a shot at doing that.
Repository:
rC Clang
https://reviews.llvm.org/D52990
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
