[Sanitizers][Darwin][Test] Remove community incompliant internal link from sources #133187
Conversation
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Mariusz Borsa (wrotki) ChangesThe malloc_zone.cpp test currently fails on Darwin hosts, in SanitizerCommon Need to XFAIL this test to buy time to investigate this failure. Also rdar://145873843 Full diff: https://github.com/llvm/llvm-project/pull/133187.diff 1 Files Affected:
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Darwin/malloc_zone.cpp b/compiler-rt/test/sanitizer_common/TestCases/Darwin/malloc_zone.cpp
index e68e93129be2f..158f788056f01 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Darwin/malloc_zone.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Darwin/malloc_zone.cpp
@@ -17,8 +17,8 @@
// UBSan does not install a malloc zone.
// XFAIL: ubsan
//
-// Curently fails on darwin/lsan rdar://145873843
-// XFAIL: darwin && lsan
+// Curently fails on darwin/lsan
+// XFAIL: (darwin && lsan) || rdar145873843
#include <malloc/malloc.h>
#include <stdlib.h>
|
|
I don't understand this CL. Isn't it already skipped with darwin && lsan? What does the |
|
|| rdar145873843 is a no-op (rdar145873843 should evaluate to 0/false). But:
Should I do it differently? Somebody spotting the XFAIL in sources might want to lookup more information on why |
|
I don't see how you are doing anything in this PR other than moving the rdar:// link into the condition? EDIT: In other words, the XFAIL you are talking about in the commit message is already there? |
|
I need to cherry-pick this XFAIL into LLVM 20 release branch, where one of our bots is failing on the test. I wanted to remove this link and also add some 'why' to the PR, so whoever merges the cherry-pick (I can't no rights) has some better context. So I decided to update the XFAIL with a separate PR - does it make sense? I know I should've done this from beginning - sorry for the confusion. |
|
I still think the PR title should represent what is actually being changed. |
wrotki
force-pushed
the
eng/m_borsa/xfail_145873843
branch
from
6b11d90 to
f715d66
Compare
wrotki
changed the title
|
That's a good point. Edited. |
|
I still don't understand. What this PR does is something like "remove rdar://145873843 link and use rdar145873843 tag instead." (FWIW, I don't necessarily agree that "|| rdar145873843" is more readable or better in really any way than just having the link there, but I don't really care about that) |
… from sources LLVM community prohibits having internal systems URL in source code. This is a correction to a previous PR XFAILing a test. The malloc_zone.cpp test currently fails on Darwin hosts, in SanitizerCommon tests with lsan enabled. Need to XFAIL this test to buy time to investigate this failure. Also we're trying to bring the number of test failing on Darwin bots to 0, to get clearer signal of any new failures. rdar://145873843
wrotki
force-pushed
the
eng/m_borsa/xfail_145873843
branch
from
f715d66 to
4d91050
Compare
wrotki
changed the title
|
Okay, now I see what you meant, sorry for being slow. Removed this rdar145873843 part too as I see how it can be confusing. |
|
@fmayer is it OK to merge it as it is now? I'd like to back port it to release/20.x too... |
|
Thanks |
The malloc_zone.cpp test currently fails on Darwin hosts, in SanitizerCommon
tests with lsan enabled.
Need to XFAIL this test to buy time to investigate this failure. Also
we're trying to bring the number of test failing on Darwin bots to 0, to
get clearer signal of any new failures.
rdar://145873843