Test sanitizers with build plans. #2720
Conversation
Lukasa
requested review from
abertelrud,
aciidgh,
friedbunny and
neonichu
as code owners
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-move-scudo-testing
branch
from
b7813eb to
0649a0d
Compare
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-move-scudo-testing
branch
from
0649a0d to
3950d7b
Compare
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-move-scudo-testing
branch
from
3950d7b to
3d99db2
Compare
|
@swift-ci please smoke test |
Lukasa
force-pushed
the
cb-move-scudo-testing
branch
from
3d99db2 to
3a27afd
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test |
|
Thanks for reworking these tests, Cory! In general, this (checking that SwiftPM is passing along the right sanitizer flag to the compiler) is definitely the right approach. One thing I'm slightly worried about is that the test seems to capture a lot of the current flags being passed to the compiler and linker, and I'm wondering if this test will frequently end up having to be modified when the generated command lines evolve over time. On one hand, because the tests specify a specific version of a manifest (v4 in this case), the semantics should stay the same even if there are changes to newer versions... but on the other hand, those semantics might in future versions be achieved through other combinations of flags. So I guess I'm wondering if there's a way to be less specific about the various flags that are getting passed (e.g. the |
|
Yeah, we can use a Swift contains operation instead if you'd like. Would that be preferable? |
|
I think so, since that seems a bit more robust in the face of future changes. Otherwise I suspect that there will need to be repeated fixes to this test over time as unrelated things change. Thanks! |
The current tests for sanitizers in SwiftPM are essentially "live": they build a package that will do something to trigger a sanitizer, and then run it and confirm it fails. This is a pretty fragile way to test, and it exposes SwiftPM to a number of possible test failures that don't have anything to do with the core mission (exposing the sanitizer). This patch replaces those tests with build plan tests that validate that SwiftPM correctly invokes swiftc and clang. These tests are acceptable because for now SwiftPM doesn't have any smarts with the sanitizers: it just passes them all through to swiftc and clang. This patch also deletes the current live tests, as they are no longer necessary.
Lukasa
force-pushed
the
cb-move-scudo-testing
branch
from
3a27afd to
d37a5c5
Compare
|
@swift-ci please smoke test |
The current tests for sanitizers in SwiftPM are essentially "live": they build a package that will do something to trigger a sanitizer, and then run it and confirm it fails. This is a pretty fragile way to test, and it exposes SwiftPM to a number of possible test failures that don't have anything to do with the core mission (exposing the sanitizer). This patch replaces those tests with build plan tests that validate that SwiftPM correctly invokes swiftc and clang. These tests are acceptable because for now SwiftPM doesn't have any smarts with the sanitizers: it just passes them all through to swiftc and clang. This patch also deletes the current live tests, as they are no longer necessary. (cherry picked from commit 984618b)
* Add support for the Scudo sanitizer. + Test sanitizers with build plans The Swift PR swiftlang/swift#28538 adds support for the Scudo hardened allocator to swiftc. This patch adds support for it to Swift Package Manager, allowing the Package Manager to correctly plumb Scudo through the entire build pipeline. See the aforementioned PR for more discussion on the value of supporting Scudo. (cherry picked from commit 594ccaa) * Test sanitizers with build plans. (#2720) The current tests for sanitizers in SwiftPM are essentially "live": they build a package that will do something to trigger a sanitizer, and then run it and confirm it fails. This is a pretty fragile way to test, and it exposes SwiftPM to a number of possible test failures that don't have anything to do with the core mission (exposing the sanitizer). This patch replaces those tests with build plan tests that validate that SwiftPM correctly invokes swiftc and clang. These tests are acceptable because for now SwiftPM doesn't have any smarts with the sanitizers: it just passes them all through to swiftc and clang. This patch also deletes the current live tests, as they are no longer necessary. (cherry picked from commit 984618b)
The current tests for sanitizers in SwiftPM are essentially "live": they
build a package that will do something to trigger a sanitizer, and then
run it and confirm it fails. This is a pretty fragile way to test, and
it exposes SwiftPM to a number of possible test failures that don't
have anything to do with the core mission (exposing the sanitizer).
This patch replaces those tests with build plan tests that validate that
SwiftPM correctly invokes swiftc and clang. These tests are acceptable
because for now SwiftPM doesn't have any smarts with the sanitizers: it
just passes them all through to swiftc and clang.
This patch also deletes the current live tests, as they are no longer
necessary.