[Runtimes][Core] Add a setting to mark backtracer path as mandatory #82842
Conversation
...and apply that to the Apple vendor caches. Addresses rdar://154230894
edymtt
requested review from
etcwilde,
shahmishal,
justice-adams-apple and
compnerd
as code owners
|
@swift-ci please smoke test |
| @@ -81,6 +81,10 @@ if(SwiftCore_ENABLE_FILESYSTEM_SUPPORT AND (NOT SwiftCore_ARCH_SUBDIR OR NOT Swi | |||
| message(SEND_ERROR "Filesystem support requires setting `SwiftCore_ARCH_SUBDIR` and `SwiftCore_PLATFORM_SUBDIR`") | |||
| endif() | |||
|
|
|||
| if(SwiftCore_VERIFY_BACKTRACER_PATH AND (NOT SwiftCore_BACKTRACER_PATH)) | |||
There was a problem hiding this comment.
I'm not sure I see the point of having this as a vendor carve out. If backtracer is enabled, always ensure that the path is provided. That simplifies the logic and reduces the number of possible configurations.
There was a problem hiding this comment.
The point is that, in general, Swift's install location isn't fixed, so the backtracer path gets constructed by the runtime by looking at the path of libswiftCore.dylib/.so/swiftCore.dll, or for -static-stdlib cases, the application's executable, and then by searching a few other "known" locations.
For Darwin specifically because it's an OS component, we don't want that behaviour, because we know where the backtracer is installed and hard-coding the path improves system security (it reduces the likelihood of an attacker being able to control the path to the backtracer).
There was a problem hiding this comment.
Why not just do that under if(APPLE) then? I think that if we can avoid options, that is better.
There was a problem hiding this comment.
Because there's at least some chance that one of the Linux distros might pick Swift up at some point, and they will want to fix the path also?
There was a problem hiding this comment.
Sure, I'm not against the SwiftCore_BACKTRACER_PATH option, I'm trying to avoid the SwiftCore_VERIFY_BACKTRACER_PATH option.
There was a problem hiding this comment.
The runtime has a default, which is convenient and necessary for testing, so there are situations where it should be left unset, like for local development. There are security implications to leaving it empty when distributing the runtimes though. Vendors should be able to set something to ensure that it can't accidentally be forgotten. If you have a better proposal than having a check for it, I'd be happy to listen.
There was a problem hiding this comment.
I now realize I started from the wrong assumption that this option is useful in general, while in fact at the moment it is specific to Darwin.
I will add this instead to our internal vendor files, so we don't add unnecessary complexity in opensource.
...and apply that to the Apple vendor caches.
Addresses rdar://154230894