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

Fri Mar 3 09:04:11 PST 2023

yln 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
+ 
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672