[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Usama Hameed via Phabricator via cfe-commits]

usama54321 added inline comments.


================
Comment at: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 
----------------
yln wrote:
> usama54321 wrote:
> > yln wrote:
> > > zixuw wrote:
> > > > dmaclach wrote:
> > > > > yln wrote:
> > > > > > This should work, right?
> > > > > No.. darwin should fail with the `-static-libsan` flag. This is the test that was failing and caused the rollback.
> > > > I think @yln is suggesting using `REQUIRES: asan-static-runtime` instead of `XFAIL: darwin`. I wasn't aware of that conditional but yeah that should be better if it works.
> > > I meant using `// REQUIRES: asan-static-runtime ` instead of `XFAIL: darwin` since it seems that we already have a lit feature for it.
> > I think UNSUPPORTED: darwin makes the most sense here. I don't think lit understands that REQUIRES: asan-static-runtime should result in skipping the test on Darwin as it does not know about this dependency.
> > I don't think lit understands that REQUIRES: asan-static-runtime should result in skipping the test on Darwin as it does not know about this dependency.
> 
> Actually, this was exactly my point.  We have other tests already marked with `REQUIRES: asan-static-runtime` and we should double check our changes don't affect these as well.
> 
> If LIT doesn't model this dependency yet, then we should make sure it does!  And this test can act as a good "canary in the coal mine".
> 
> Please use `REQUIRES: asan-static-runtime` and make sure we understand and deal with any fallout.
Sorry for my earlier comment. @yln is correct, and we should use REQUIRES: asan-static-runtime. I double checked, and this already works as expected on darwin, i.e. these tests are unsupported on darwin. So you should not need to do anything apart from adding the REQUIRES in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672