[PATCH] D147444: [asan][test][win] Port more tests to not use clang-cl on MinGW

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: compiler-rt/test/asan/TestCases/Windows/crt_initializers.cpp:1
-// UNSUPPORTED: target={{.*-windows-gnu}}
-
-// RUN: %clang_cl_asan -Od %s -Fe%t
+// RUN: %if clang-cl %{ %clang_cl_asan -Od %s -Fe%t %} \
+// RUN: %else %{ %clangxx_asan -O0 %s -o %t %}
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > vitalybuka wrote:
> > > mstorsjo wrote:
> > > > alvinhochun wrote:
> > > > > mstorsjo wrote:
> > > > > > alvinhochun wrote:
> > > > > > > vitalybuka wrote:
> > > > > > > > I believe it would be much cleaner if done substituting: llvm-project/compiler-rt/test/asan/lit.cfg.py:163
> > > > > > > > 
> > > > > > > I don't know how one could use substitution for this, since clang and clang-cl use very different command line parameters (especially problematic for the more complex DLL tests which I have yet to port).
> > > > > > > 
> > > > > > > Or do you mean using substitution to turn certain `RUN:` commands into comments? Like:
> > > > > > > 
> > > > > > > ```
> > > > > > > // RUN: %if_clang_cl %clang_cl_asan -Od %s -Fe%t
> > > > > > > // RUN: %if_not_clang_cl %clangxx_asan -O0 %s -o %t
> > > > > > > ```
> > > > > > > 
> > > > > > > ...and conditionally substituting one of `%if_clang_cl` and `%if_not_clang_cl` to be `:` and the other one to be empty?
> > > > > > You could make one substitution for `-Fe` or `-o ` (note, including the trailing space), another one for `-O0` vs `-Od` etc.
> > > > > > 
> > > > > > Sorry if it’s obvious if I haven’t followed closely enough, but would it be possible to port the tests to use the gcc driver syntax even with clang in msvc mode? (I understand that that would be a bigger undertaking though.) Or are these tests executed with cl.exe too, so we need to remain compatible with that syntax anyway?
> > > > > If we need one substitution per flag, I don't know how many substitutions we will end up having, and I may have difficulty naming them. Some flags may also be incompatible (mingw needs `-Wl,--out-implib` to generate import lib). To me it feels messier than just writing the commands separately.
> > > > > 
> > > > > Rewriting to use gcc driver syntax shouldn't be hard, but I am afraid of breaking them due to obscure differences between the two drivers (things like static/dynamic, debug/release crt). I suppose these tests also serve the purpose of testing asan support in the clang-cl driver so perhaps they should continue to use clang-cl.
> > > > > 
> > > > > It doesn't look like the tests would work with cl.exe out of the box though.
> > > > @vitalybuka - Can you elaborate on how you want this to be substituted?
> > > Per flag, like %fPIC llvm-project/compiler-rt/test/asan/lit.cfg.py:234 ?
> > at least for frequent stuff
> > for flags which are in 1-2 tests if/else should be fine
> > 
> @phosek @MaskRay Do you have opinion on %if %else in run lines?
> Maybe I am unfairly biased against them, when they are reasonable way to solve the problem?
> 
I'll certainly avoid complex constructs if possible.
When they provide values, I'll not simply keep them off. 

`rg '%if target=' compiler-rt/test/` has some examples.

Bonus: our internal lit runner supports many `%if` constructs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147444/new/

https://reviews.llvm.org/D147444