-
Notifications
You must be signed in to change notification settings - Fork 14k
Revert "[LLDB][ELF Core] Support all the Generic (Negative) SI Codes." #141645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) ChangesReverts llvm/llvm-project#140150 Broke the Darwin tests, but they pass on Linux. Reverting to make the build healthy while I investigate Patch is 38.68 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141645.diff 10 Files Affected:
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 35ffdabf907e7..a702abb540fd9 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -21,7 +21,6 @@
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/File.h"
#include "lldb/Interpreter/Options.h"
-#include "lldb/Target/StopInfo.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/FileSpec.h"
@@ -961,8 +960,6 @@ class Platform : public PluginInterface {
virtual CompilerType GetSiginfoType(const llvm::Triple &triple);
- virtual lldb::StopInfoSP GetStopInfoFromSiginfo(Thread &thread) { return {}; }
-
virtual Args GetExtraStartupCommands();
typedef std::function<Status(const ModuleSpec &module_spec,
diff --git a/lldb/include/lldb/Target/UnixSignals.h b/lldb/include/lldb/Target/UnixSignals.h
index a1807d69f329b..b3605ccefddbe 100644
--- a/lldb/include/lldb/Target/UnixSignals.h
+++ b/lldb/include/lldb/Target/UnixSignals.h
@@ -36,9 +36,7 @@ class UnixSignals {
std::optional<int32_t> code = std::nullopt,
std::optional<lldb::addr_t> addr = std::nullopt,
std::optional<lldb::addr_t> lower = std::nullopt,
- std::optional<lldb::addr_t> upper = std::nullopt,
- std::optional<uint32_t> pid = std::nullopt,
- std::optional<uint32_t> uid = std::nullopt) const;
+ std::optional<lldb::addr_t> upper = std::nullopt) const;
bool SignalIsValid(int32_t signo) const;
@@ -107,7 +105,7 @@ class UnixSignals {
llvm::StringRef description,
llvm::StringRef alias = llvm::StringRef());
- enum SignalCodePrintOption { None, Address, Bounds, Sender };
+ enum SignalCodePrintOption { None, Address, Bounds };
// Instead of calling this directly, use a ADD_SIGCODE macro to get compile
// time checks when on the native platform.
diff --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
index cb60caf1cb422..9db2c83acc125 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -14,7 +14,6 @@
#include <sys/utsname.h>
#endif
-#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Utility/ARM64_DWARF_Registers.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
@@ -481,107 +480,3 @@ CompilerType PlatformLinux::GetSiginfoType(const llvm::Triple &triple) {
ast->CompleteTagDeclarationDefinition(siginfo_type);
return siginfo_type;
}
-
-static std::string GetDescriptionFromSiginfo(lldb::ValueObjectSP siginfo_sp) {
- if (!siginfo_sp)
- return "";
-
- lldb_private::LinuxSignals linux_signals;
- int code = siginfo_sp->GetChildMemberWithName("si_code")->GetValueAsSigned(0);
- int signo =
- siginfo_sp->GetChildMemberWithName("si_signo")->GetValueAsSigned(-1);
-
- auto sifields = siginfo_sp->GetChildMemberWithName("_sifields");
- if (!sifields)
- return linux_signals.GetSignalDescription(signo, code);
-
- // declare everything that we can populate later.
- std::optional<lldb::addr_t> addr;
- std::optional<lldb::addr_t> upper;
- std::optional<lldb::addr_t> lower;
- std::optional<uint32_t> pid;
- std::optional<uint32_t> uid;
-
- // The negative si_codes are special and mean this signal was sent from user
- // space not the kernel. These take precedence because they break some of the
- // invariants around kernel sent signals. Such as SIGSEGV won't have an
- // address.
- if (code < 0) {
- auto sikill = sifields->GetChildMemberWithName("_kill");
- if (sikill) {
- auto pid_sp = sikill->GetChildMemberWithName("si_pid");
- if (pid_sp)
- pid = pid_sp->GetValueAsUnsigned(-1);
- auto uid_sp = sikill->GetChildMemberWithName("si_uid");
- if (uid_sp)
- uid = uid_sp->GetValueAsUnsigned(-1);
- }
- } else {
-
- switch (signo) {
- case SIGILL:
- case SIGFPE:
- case SIGBUS: {
- auto sigfault = sifields->GetChildMemberWithName("_sigfault");
- if (!sigfault)
- break;
-
- auto addr_sp = sigfault->GetChildMemberWithName("si_addr");
- if (addr_sp)
- addr = addr_sp->GetValueAsUnsigned(-1);
- break;
- }
- case SIGSEGV: {
- auto sigfault = sifields->GetChildMemberWithName("_sigfault");
- if (!sigfault)
- break;
-
- auto addr_sp = sigfault->GetChildMemberWithName("si_addr");
- if (addr_sp)
- addr = addr_sp->GetValueAsUnsigned(-1);
-
- auto bounds_sp = sigfault->GetChildMemberWithName("_bounds");
- if (!bounds_sp)
- break;
-
- auto addr_bnds_sp = bounds_sp->GetChildMemberWithName("_addr_bnd");
- if (!addr_bnds_sp)
- break;
-
- auto lower_sp = addr_bnds_sp->GetChildMemberWithName("_lower");
- if (lower_sp)
- lower = lower_sp->GetValueAsUnsigned(-1);
-
- auto upper_sp = addr_bnds_sp->GetChildMemberWithName("_upper");
- if (upper_sp)
- upper = upper_sp->GetValueAsUnsigned(-1);
-
- break;
- }
- default:
- break;
- }
- }
-
- return linux_signals.GetSignalDescription(signo, code, addr, lower, upper,
- uid, pid);
-}
-
-lldb::StopInfoSP PlatformLinux::GetStopInfoFromSiginfo(Thread &thread) {
- ValueObjectSP siginfo_sp = thread.GetSiginfoValue();
- if (!siginfo_sp)
- return {};
- auto signo_sp = siginfo_sp->GetChildMemberWithName("si_signo");
- auto sicode_sp = siginfo_sp->GetChildMemberWithName("si_code");
- if (!signo_sp || !sicode_sp)
- return {};
-
- std::string siginfo_description = GetDescriptionFromSiginfo(siginfo_sp);
- if (siginfo_description.empty())
- return StopInfo::CreateStopReasonWithSignal(
- thread, signo_sp->GetValueAsUnsigned(-1));
-
- return StopInfo::CreateStopReasonWithSignal(
- thread, signo_sp->GetValueAsUnsigned(-1), siginfo_description.c_str(),
- sicode_sp->GetValueAsUnsigned(0));
-}
diff --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
index 0fd33b03dcd23..89f0bd709ef60 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
@@ -62,8 +62,6 @@ class PlatformLinux : public PlatformPOSIX {
CompilerType GetSiginfoType(const llvm::Triple &triple) override;
- lldb::StopInfoSP GetStopInfoFromSiginfo(Thread &thread) override;
-
std::vector<ArchSpec> m_supported_architectures;
private:
diff --git a/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp b/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
index 76c32e376eb4b..9c4fe55147a28 100644
--- a/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
+++ b/lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
@@ -38,28 +38,6 @@
#define ADD_SIGCODE(signal_name, signal_value, code_name, code_value, ...) \
AddSignalCode(signal_value, code_value, __VA_ARGS__)
#endif /* if defined(__linux__) && !defined(__mips__) */
-// See siginfo.h in the Linux Kernel, these codes can be sent for any signal.
-#define ADD_LINUX_SIGNAL(signo, name, ...) \
- AddSignal(signo, name, __VA_ARGS__); \
- ADD_SIGCODE(signo, signo, SI_QUEUE, -1, "sent by sigqueue", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_TIMER, -2, "sent by timer expiration", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_MESGQ, -3, \
- "sent by real time mesq state change", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_ASYNCIO, -4, "sent by AIO completion", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_SIGIO, -5, "sent by queued SIGIO", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_TKILL, -6, "sent by tkill system call", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_DETHREAD, -7, \
- "sent by execve() killing subsidiary threads", \
- SignalCodePrintOption::Sender); \
- ADD_SIGCODE(signo, signo, SI_ASYNCNL, -60, \
- "sent by glibc async name lookup completion", \
- SignalCodePrintOption::Sender);
using namespace lldb_private;
@@ -68,13 +46,13 @@ LinuxSignals::LinuxSignals() : UnixSignals() { Reset(); }
void LinuxSignals::Reset() {
m_signals.clear();
// clang-format off
- // SIGNO NAME SUPPRESS STOP NOTIFY DESCRIPTION
- // ====== ============== ======== ====== ====== ===================================================
- ADD_LINUX_SIGNAL(1, "SIGHUP", false, true, true, "hangup");
- ADD_LINUX_SIGNAL(2, "SIGINT", true, true, true, "interrupt");
- ADD_LINUX_SIGNAL(3, "SIGQUIT", false, true, true, "quit");
+ // SIGNO NAME SUPPRESS STOP NOTIFY DESCRIPTION
+ // ====== ============== ======== ====== ====== ===================================================
+ AddSignal(1, "SIGHUP", false, true, true, "hangup");
+ AddSignal(2, "SIGINT", true, true, true, "interrupt");
+ AddSignal(3, "SIGQUIT", false, true, true, "quit");
- ADD_LINUX_SIGNAL(4, "SIGILL", false, true, true, "illegal instruction");
+ AddSignal(4, "SIGILL", false, true, true, "illegal instruction");
ADD_SIGCODE(SIGILL, 4, ILL_ILLOPC, 1, "illegal opcode");
ADD_SIGCODE(SIGILL, 4, ILL_ILLOPN, 2, "illegal operand");
ADD_SIGCODE(SIGILL, 4, ILL_ILLADR, 3, "illegal addressing mode");
@@ -84,15 +62,15 @@ void LinuxSignals::Reset() {
ADD_SIGCODE(SIGILL, 4, ILL_COPROC, 7, "coprocessor error");
ADD_SIGCODE(SIGILL, 4, ILL_BADSTK, 8, "internal stack error");
- ADD_LINUX_SIGNAL(5, "SIGTRAP", true, true, true, "trace trap (not reset when caught)");
- ADD_LINUX_SIGNAL(6, "SIGABRT", false, true, true, "abort()/IOT trap", "SIGIOT");
+ AddSignal(5, "SIGTRAP", true, true, true, "trace trap (not reset when caught)");
+ AddSignal(6, "SIGABRT", false, true, true, "abort()/IOT trap", "SIGIOT");
- ADD_LINUX_SIGNAL(7, "SIGBUS", false, true, true, "bus error");
+ AddSignal(7, "SIGBUS", false, true, true, "bus error");
ADD_SIGCODE(SIGBUS, 7, BUS_ADRALN, 1, "illegal alignment");
ADD_SIGCODE(SIGBUS, 7, BUS_ADRERR, 2, "illegal address");
ADD_SIGCODE(SIGBUS, 7, BUS_OBJERR, 3, "hardware error");
- ADD_LINUX_SIGNAL(8, "SIGFPE", false, true, true, "floating point exception");
+ AddSignal(8, "SIGFPE", false, true, true, "floating point exception");
ADD_SIGCODE(SIGFPE, 8, FPE_INTDIV, 1, "integer divide by zero");
ADD_SIGCODE(SIGFPE, 8, FPE_INTOVF, 2, "integer overflow");
ADD_SIGCODE(SIGFPE, 8, FPE_FLTDIV, 3, "floating point divide by zero");
@@ -102,10 +80,10 @@ void LinuxSignals::Reset() {
ADD_SIGCODE(SIGFPE, 8, FPE_FLTINV, 7, "floating point invalid operation");
ADD_SIGCODE(SIGFPE, 8, FPE_FLTSUB, 8, "subscript out of range");
- ADD_LINUX_SIGNAL(9, "SIGKILL", false, true, true, "kill");
- ADD_LINUX_SIGNAL(10, "SIGUSR1", false, true, true, "user defined signal 1");
+ AddSignal(9, "SIGKILL", false, true, true, "kill");
+ AddSignal(10, "SIGUSR1", false, true, true, "user defined signal 1");
- ADD_LINUX_SIGNAL(11, "SIGSEGV", false, true, true, "segmentation violation");
+ AddSignal(11, "SIGSEGV", false, true, true, "segmentation violation");
ADD_SIGCODE(SIGSEGV, 11, SEGV_MAPERR, 1, "address not mapped to object", SignalCodePrintOption::Address);
ADD_SIGCODE(SIGSEGV, 11, SEGV_ACCERR, 2, "invalid permissions for mapped object", SignalCodePrintOption::Address);
ADD_SIGCODE(SIGSEGV, 11, SEGV_BNDERR, 3, "failed address bounds checks", SignalCodePrintOption::Bounds);
@@ -116,58 +94,58 @@ void LinuxSignals::Reset() {
// codes. One way to get this is via unaligned SIMD loads. Treat it as invalid address.
ADD_SIGCODE(SIGSEGV, 11, SI_KERNEL, 0x80, "invalid address", SignalCodePrintOption::Address);
- ADD_LINUX_SIGNAL(12, "SIGUSR2", false, true, true, "user defined signal 2");
- ADD_LINUX_SIGNAL(13, "SIGPIPE", false, true, true, "write to pipe with reading end closed");
- ADD_LINUX_SIGNAL(14, "SIGALRM", false, false, false, "alarm");
- ADD_LINUX_SIGNAL(15, "SIGTERM", false, true, true, "termination requested");
- ADD_LINUX_SIGNAL(16, "SIGSTKFLT", false, true, true, "stack fault");
- ADD_LINUX_SIGNAL(17, "SIGCHLD", false, false, true, "child status has changed", "SIGCLD");
- ADD_LINUX_SIGNAL(18, "SIGCONT", false, false, true, "process continue");
- ADD_LINUX_SIGNAL(19, "SIGSTOP", true, true, true, "process stop");
- ADD_LINUX_SIGNAL(20, "SIGTSTP", false, true, true, "tty stop");
- ADD_LINUX_SIGNAL(21, "SIGTTIN", false, true, true, "background tty read");
- ADD_LINUX_SIGNAL(22, "SIGTTOU", false, true, true, "background tty write");
- ADD_LINUX_SIGNAL(23, "SIGURG", false, true, true, "urgent data on socket");
- ADD_LINUX_SIGNAL(24, "SIGXCPU", false, true, true, "CPU resource exceeded");
- ADD_LINUX_SIGNAL(25, "SIGXFSZ", false, true, true, "file size limit exceeded");
- ADD_LINUX_SIGNAL(26, "SIGVTALRM", false, true, true, "virtual time alarm");
- ADD_LINUX_SIGNAL(27, "SIGPROF", false, false, false, "profiling time alarm");
- ADD_LINUX_SIGNAL(28, "SIGWINCH", false, true, true, "window size changes");
- ADD_LINUX_SIGNAL(29, "SIGIO", false, true, true, "input/output ready/Pollable event", "SIGPOLL");
- ADD_LINUX_SIGNAL(30, "SIGPWR", false, true, true, "power failure");
- ADD_LINUX_SIGNAL(31, "SIGSYS", false, true, true, "invalid system call");
- ADD_LINUX_SIGNAL(32, "SIG32", false, false, false, "threading library internal signal 1");
- ADD_LINUX_SIGNAL(33, "SIG33", false, false, false, "threading library internal signal 2");
- ADD_LINUX_SIGNAL(34, "SIGRTMIN", false, false, false, "real time signal 0");
- ADD_LINUX_SIGNAL(35, "SIGRTMIN+1", false, false, false, "real time signal 1");
- ADD_LINUX_SIGNAL(36, "SIGRTMIN+2", false, false, false, "real time signal 2");
- ADD_LINUX_SIGNAL(37, "SIGRTMIN+3", false, false, false, "real time signal 3");
- ADD_LINUX_SIGNAL(38, "SIGRTMIN+4", false, false, false, "real time signal 4");
- ADD_LINUX_SIGNAL(39, "SIGRTMIN+5", false, false, false, "real time signal 5");
- ADD_LINUX_SIGNAL(40, "SIGRTMIN+6", false, false, false, "real time signal 6");
- ADD_LINUX_SIGNAL(41, "SIGRTMIN+7", false, false, false, "real time signal 7");
- ADD_LINUX_SIGNAL(42, "SIGRTMIN+8", false, false, false, "real time signal 8");
- ADD_LINUX_SIGNAL(43, "SIGRTMIN+9", false, false, false, "real time signal 9");
- ADD_LINUX_SIGNAL(44, "SIGRTMIN+10", false, false, false, "real time signal 10");
- ADD_LINUX_SIGNAL(45, "SIGRTMIN+11", false, false, false, "real time signal 11");
- ADD_LINUX_SIGNAL(46, "SIGRTMIN+12", false, false, false, "real time signal 12");
- ADD_LINUX_SIGNAL(47, "SIGRTMIN+13", false, false, false, "real time signal 13");
- ADD_LINUX_SIGNAL(48, "SIGRTMIN+14", false, false, false, "real time signal 14");
- ADD_LINUX_SIGNAL(49, "SIGRTMIN+15", false, false, false, "real time signal 15");
- ADD_LINUX_SIGNAL(50, "SIGRTMAX-14", false, false, false, "real time signal 16"); // switching to SIGRTMAX-xxx to match "kill -l" output
- ADD_LINUX_SIGNAL(51, "SIGRTMAX-13", false, false, false, "real time signal 17");
- ADD_LINUX_SIGNAL(52, "SIGRTMAX-12", false, false, false, "real time signal 18");
- ADD_LINUX_SIGNAL(53, "SIGRTMAX-11", false, false, false, "real time signal 19");
- ADD_LINUX_SIGNAL(54, "SIGRTMAX-10", false, false, false, "real time signal 20");
- ADD_LINUX_SIGNAL(55, "SIGRTMAX-9", false, false, false, "real time signal 21");
- ADD_LINUX_SIGNAL(56, "SIGRTMAX-8", false, false, false, "real time signal 22");
- ADD_LINUX_SIGNAL(57, "SIGRTMAX-7", false, false, false, "real time signal 23");
- ADD_LINUX_SIGNAL(58, "SIGRTMAX-6", false, false, false, "real time signal 24");
- ADD_LINUX_SIGNAL(59, "SIGRTMAX-5", false, false, false, "real time signal 25");
- ADD_LINUX_SIGNAL(60, "SIGRTMAX-4", false, false, false, "real time signal 26");
- ADD_LINUX_SIGNAL(61, "SIGRTMAX-3", false, false, false, "real time signal 27");
- ADD_LINUX_SIGNAL(62, "SIGRTMAX-2", false, false, false, "real time signal 28");
- ADD_LINUX_SIGNAL(63, "SIGRTMAX-1", false, false, false, "real time signal 29");
- ADD_LINUX_SIGNAL(64, "SIGRTMAX", false, false, false, "real time signal 30");
+ AddSignal(12, "SIGUSR2", false, true, true, "user defined signal 2");
+ AddSignal(13, "SIGPIPE", false, true, true, "write to pipe with reading end closed");
+ AddSignal(14, "SIGALRM", false, false, false, "alarm");
+ AddSignal(15, "SIGTERM", false, true, true, "termination requested");
+ AddSignal(16, "SIGSTKFLT", false, true, true, "stack fault");
+ AddSignal(17, "SIGCHLD", false, false, true, "child status has changed", "SIGCLD");
+ AddSignal(18, "SIGCONT", false, false, true, "process continue");
+ AddSignal(19, "SIGSTOP", true, true, true, "process stop");
+ AddSignal(20, "SIGTSTP", false, true, true, "tty stop");
+ AddSignal(21, "SIGTTIN", false, true, true, "background tty read");
+ AddSignal(22, "SIGTTOU", false, true, true, "background tty write");
+ AddSignal(23, "SIGURG", false, true, true, "urgent data on socket");
+ AddSignal(24, "SIGXCPU", false, true, true, "CPU resource exceeded");
+ AddSignal(25, "SIGXFSZ", false, true, true, "file size limit exceeded");
+ AddSignal(26, "SIGVTALRM", false, true, true, "virtual time alarm");
+ AddSignal(27, "SIGPROF", false, false, false, "profiling time alarm");
+ AddSignal(28, "SIGWINCH", false, true, true, "window size changes");
+ AddSignal(29, "SIGIO", false, true, true, "input/output ready/Pollable event", "SIGPOLL");
+ AddSignal(30, "SIGPWR", false, true, true, "power failure");
+ AddSignal(31, "SIGSYS", false, true, true, "invalid system call");
+ AddSignal(32, "SIG32", false, false, false, "threading library internal signal 1");
+ AddSignal(33, "SIG33", false, false, false, "threading library internal signal 2");
+ AddSignal(34, "SIGRTMIN", false, false, false, "real time signal 0");
+ AddSignal(35, "SIGRTMIN+1", false, false, false, "real time signal 1");
+ AddSignal(36, "SIGRTMIN+2", false, false, false, "real time signal 2");
+ AddSignal(37, "SIGRTMIN+3", false, false, false, "real time signal 3");
+ AddSignal(38, "SIGRTMIN+4", false, false, false, "real time signa...
[truncated]
|
Jlalond
added a commit
to Jlalond/llvm-project
that referenced
this pull request
May 27, 2025
…." (llvm#141645) This reverts commit 9d33b92.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
llvm#141645) Reverts llvm#140150 Broke the Darwin tests, but they pass on Linux. Reverting to make the build healthy while I investigate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #140150
Broke the Darwin tests, but they pass on Linux. Reverting to make the build healthy while I investigate