Uh oh!
There was an error while loading. Please reload this page.
Fix FreeBSD/PowerPC* issues - #2147
Open
kgotlinux wants to merge 2 commits into
Open
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
StacktracePowerPCGetLRPtr() falls back to OS checks when the compiler does not provide the _CALL_* ABI macros. Clang defines neither _CALL_SYSV nor _CALL_AIX on any FreeBSD PowerPC target (64-bit gets only _CALL_ELF=2), so 32-bit powerpc-*-freebsd hits the __FreeBSD__ fallback and reads the LR from sp[2]. The 32-bit SVR4 ABI saves the LR in the second word of the frame, sp[1]; sp[2] is correct only for the 64-bit ELF ABIs and Darwin/AIX. Verified against clang codegen: powerpc-unknown-freebsd emits "stwu 1,-16(1); stw 0,20(1)" (old SP + 4) while powerpc64-unknown-freebsd emits "stdu 1,-48(1); std 0,64(1)" (old SP + 16). Key the fallback on the word size instead of the OS: any __PPC64__ target uses sp[2], and 32-bit ELF targets (Linux, FreeBSD) use sp[1]. This is behavior-preserving for Linux and for 64-bit FreeBSD, and fixes the 32-bit FreeBSD case (currently latent, since stacktrace_config.h does not select this unwinder on FreeBSD).
The FreeBSD branch reads the kern.timecounter.tc.timebase.frequency sysctl straight into a double. The kernel registers that OID as CTLTYPE_U64 (sys/kern/kern_tc.c, tc_init()), so sysctlbyname() fills the 8-byte buffer with the integer 512000000 and the bits are then interpreted as an IEEE double: Frequency() returns ~2.5e-315 instead of 5.12e+08. The size check cannot catch this because uint64_t and double are both 8 bytes. Observed on FreeBSD 15.1 powerpc64le and powerpc64 (POWER9): - LOG_EVERY_N_SEC / VLOG_EVERY_N_SEC never rate-limit, because LogEveryNSecState::ShouldLog() computes the next log time as now + seconds * Frequency() ~= now (1000 of 1000 iterations logged with a 3600 s period). - Mutex waiters refresh their pthread_getschedparam() cache on every block instead of once per second. Read the sysctl into a uint64_t and convert to double; keep 0.0 if the sysctl fails.
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
Sign up for freeto 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.
This is a cumulative PR for two FreeBSD/PowerPC* changes: