Skip to content

Commit

Permalink
Bug 1856297 - Remove the CrashAddressLikelyWrong annotation and its…
Browse files Browse the repository at this point in the history
… associated machinery r=glandium

Differential Revision: https://phabricator.services.mozilla.com/D189758
  • Loading branch information
gabrielesvelto committed Oct 4, 2023
1 parent b8576de commit c161434
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 64 deletions.
11 changes: 0 additions & 11 deletions js/src/wasm/WasmSignalHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -801,10 +801,6 @@ static void WasmTrapHandler(int signum, siginfo_t* info, void* context) {
}
# endif // XP_WIN || XP_DARWIN || assume unix

# if defined(ANDROID) && defined(MOZ_LINKER)
extern "C" MFBT_API bool IsSignalHandlingBroken();
# endif

struct InstallState {
bool tried;
bool success;
Expand All @@ -829,13 +825,6 @@ void wasm::EnsureEagerProcessSignalHandlers() {
eagerInstallState->tried = true;
MOZ_RELEASE_ASSERT(eagerInstallState->success == false);

# if defined(ANDROID) && defined(MOZ_LINKER)
// Signal handling is broken on some android systems.
if (IsSignalHandlingBroken()) {
return;
}
# endif

sAlreadyHandlingTrap.infallibleInit();

// Install whatever exception/signal handler is appropriate for the OS.
Expand Down
41 changes: 8 additions & 33 deletions mozglue/linker/ElfLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,7 @@ static int GetAndroidSDKVersion() {
return version;
}

# if __ANDROID_API__ < 8
/* Android API < 8 doesn't provide sigaltstack */

extern "C" {

inline int sigaltstack(const stack_t* ss, stack_t* oss) {
return syscall(__NR_sigaltstack, ss, oss);
}

} /* extern "C" */
# endif /* __ANDROID_API__ */
#endif /* ANDROID */
#endif /* ANDROID */

#ifdef __ARM_EABI__
extern "C" MOZ_EXPORT const void* __gnu_Unwind_Find_exidx(void* pc, int* pcount)
Expand Down Expand Up @@ -315,10 +304,6 @@ MFBT_API void __dl_munmap(void* handle, void* addr, size_t length) {
return reinterpret_cast<LibHandle*>(handle)->MappableMUnmap(addr, length);
}

MFBT_API bool IsSignalHandlingBroken() {
return ElfLoader::Singleton.isSignalHandlingBroken();
}

namespace {

/**
Expand Down Expand Up @@ -1164,10 +1149,7 @@ struct TmpData {
};

SEGVHandler::SEGVHandler()
: initialized(false),
registeredHandler(false),
signalHandlingBroken(true),
signalHandlingSlow(true) {
: initialized(false), registeredHandler(false), signalHandlingSlow(true) {
/* Ensure logging is initialized before the DEBUG_LOG in the test_handler.
* As this constructor runs before the ElfLoader constructor (by effect
* of ElfLoader inheriting from this class), this also initializes on behalf
Expand All @@ -1184,14 +1166,9 @@ SEGVHandler::SEGVHandler()
struct sigaction old_action;
sys_sigaction(SIGSEGV, nullptr, &old_action);

/* Some devices don't provide useful information to their SIGSEGV handlers,
* making it impossible for on-demand decompression to work. To check if
* we're on such a device, setup a temporary handler and deliberately
* trigger a segfault. The handler will set signalHandlingBroken if the
* provided information is bogus.
* Some other devices have a kernel option enabled that makes SIGSEGV handler
/* Some devices have a kernel option enabled that makes SIGSEGV handler
* have an overhead so high that it affects how on-demand decompression
* performs. The handler will also set signalHandlingSlow if the triggered
* performs. The handler will set signalHandlingSlow if the triggered
* SIGSEGV took too much time. */
struct sigaction action;
action.sa_sigaction = &SEGVHandler::test_handler;
Expand All @@ -1217,7 +1194,9 @@ void SEGVHandler::FinishInitialization() {
* going to race with another thread. */
initialized = true;

if (signalHandlingBroken || signalHandlingSlow) return;
if (signalHandlingSlow) {
return;
}

typedef int (*sigaction_func)(int, const struct sigaction*,
struct sigaction*);
Expand Down Expand Up @@ -1304,13 +1283,9 @@ SEGVHandler::~SEGVHandler() {
}

/* Test handler for a deliberately triggered SIGSEGV that determines whether
* useful information is provided to signal handlers, particularly whether
* si_addr is filled in properly, and whether the segfault handler is called
* quickly enough. */
* the segfault handler is called quickly enough. */
void SEGVHandler::test_handler(int signum, siginfo_t* info, void* context) {
SEGVHandler& that = ElfLoader::Singleton;
if (signum == SIGSEGV && info && info->si_addr == that.stackPtr.get())
that.signalHandlingBroken = false;
mprotect(that.stackPtr, that.stackPtr.GetLength(), PROT_READ | PROT_WRITE);
TmpData* data = reinterpret_cast<TmpData*>(that.stackPtr.get());
uint64_t latency = ProcessTimeStamp_Now() - data->crash_timestamp;
Expand Down
5 changes: 0 additions & 5 deletions mozglue/linker/ElfLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ MFBT_API size_t __dl_get_mappable_length(void* handle);
MFBT_API void* __dl_mmap(void* handle, void* addr, size_t length, off_t offset);

MFBT_API void __dl_munmap(void* handle, void* addr, size_t length);

MFBT_API bool IsSignalHandlingBroken();
}

/* Forward declarations for use in LibHandle */
Expand Down Expand Up @@ -316,8 +314,6 @@ class SEGVHandler {
return registeredHandler;
}

bool isSignalHandlingBroken() { return signalHandlingBroken; }

static int __wrap_sigaction(int signum, const struct sigaction* act,
struct sigaction* oldact);

Expand Down Expand Up @@ -366,7 +362,6 @@ class SEGVHandler {

bool initialized;
bool registeredHandler;
bool signalHandlingBroken;
bool signalHandlingSlow;
};

Expand Down
6 changes: 0 additions & 6 deletions toolkit/crashreporter/CrashAnnotations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,6 @@ CPUMicrocodeVersion:
Version of the CPU microcode.
type: string

CrashAddressLikelyWrong:
description: >
Set to 1 if signal handling is broken, in which case the crash address is
likely to be wrong.
type: boolean

CrashTime:
description: >
Crash time in seconds since the Epoch.
Expand Down
9 changes: 0 additions & 9 deletions toolkit/xre/nsAppRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,6 @@ nsString gProcessStartupShortcut;
#endif
#include "BinaryPath.h"

#ifdef MOZ_LINKER
extern "C" MFBT_API bool IsSignalHandlingBroken();
#endif

#ifdef FUZZING
# include "FuzzerRunner.h"

Expand Down Expand Up @@ -4148,11 +4144,6 @@ int XREMain::XRE_mainInit(bool* aExitFlag) {
nsDependentCString releaseChannel(MOZ_STRINGIFY(MOZ_UPDATE_CHANNEL));
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::ReleaseChannel, releaseChannel);
#ifdef MOZ_LINKER
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::CrashAddressLikelyWrong,
IsSignalHandlingBroken());
#endif

#ifdef XP_WIN
nsAutoString appInitDLLs;
Expand Down

0 comments on commit c161434

Please sign in to comment.