Bug 1779257 - Fix the stack walking for linux markers by calling getcontext in a valid stack frame r=mstange

See my comment on here for more context of my investigation:
https://bugzilla.mozilla.org/show_bug.cgi?id=1779257#c9

The saved context is invalid once the function that called `getcontext`
returns. We need to call the `getcontext` while the frame where we called it is
still on the stack. That's why this patch is moving the call to `getcontext` to
parent function by inlining the SyncPopulate content by using a macro instead.
This has to be a macro instead of a function because stack pointer address will
be invalid once the `Registers::SyncPopulate` returns. I tried to change this
method to inline but that didn't help either.

Differential Revision: https://phabricator.services.mozilla.com/D170133

memory/replace/dmd/DMD.cpp

@@ -636,7 +636,7 @@ static uint32_t gGCStackTraceTableWhenSizeExceeds = 4 * 1024;
     // when it is called during static initialization (see bug 1241684).
     //
     // This code is cribbed from the Gecko Profiler, which also uses
-    // FramePointerStackWalk() on Win32: Registers::SyncPopulate() for the
+    // FramePointerStackWalk() on Win32: REGISTERS_SYNC_POPULATE() for the
     // frame pointer, and GetStackTop() for the stack end.
     CONTEXT context;
     RtlCaptureContext(&context);
@@ -650,7 +650,7 @@ static uint32_t gGCStackTraceTableWhenSizeExceeds = 4 * 1024;
     // changes in libunwind.
     //
     // This code is cribbed from the Gecko Profiler, which also uses
-    // FramePointerStackWalk() on Mac: Registers::SyncPopulate() for the frame
+    // FramePointerStackWalk() on Mac: REGISTERS_SYNC_POPULATE() for the frame
     // pointer, and GetStackTop() for the stack end.
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wframe-address"

mozglue/baseprofiler/core/platform-linux-android.cpp

@@ -498,18 +498,9 @@ static void PlatformInit(PSLockRef aLock) {}
 #endif
 
 #if defined(HAVE_NATIVE_UNWIND)
-// Context used by synchronous samples. It's safe to have a single one because
-// only one synchronous sample can be taken at a time (due to
-// profiler_get_backtrace()'s PSAutoLock).
-// ucontext_t sSyncUContext;
-
-void Registers::SyncPopulate() {
-  // TODO port getcontext from breakpad, if profiler_get_backtrace is needed.
-  MOZ_CRASH("profiler_get_backtrace() unsupported");
-  // if (!getcontext(&sSyncUContext)) {
-  //   PopulateRegsFromContext(*this, &sSyncUContext);
-  // }
-}
+// TODO port getcontext from breakpad, if profiler_get_backtrace is needed.
+#  define REGISTERS_SYNC_POPULATE(regs) \
+    MOZ_CRASH("profiler_get_backtrace() unsupported");
 #endif
 
 }  // namespace baseprofiler

mozglue/baseprofiler/core/platform-macos.cpp

@@ -198,22 +198,23 @@ void SamplerThread::Stop(PSLockRef aLock) { mSampler.Disable(aLock); }
 
 static void PlatformInit(PSLockRef aLock) {}
 
+// clang-format off
 #if defined(HAVE_NATIVE_UNWIND)
-void Registers::SyncPopulate() {
-  // Derive the stack pointer from the frame pointer. The 0x10 offset is
-  // 8 bytes for the previous frame pointer and 8 bytes for the return
-  // address both stored on the stack after at the beginning of the current
-  // frame.
-  mSP = reinterpret_cast<Address>(__builtin_frame_address(0)) + 0x10;
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wframe-address"
-  mFP = reinterpret_cast<Address>(__builtin_frame_address(1));
-#  pragma GCC diagnostic pop
-  mPC = reinterpret_cast<Address>(
-      __builtin_extract_return_addr(__builtin_return_address(0)));
-  mLR = 0;
-}
+// Derive the stack pointer from the frame pointer. The 0x10 offset is
+// 8 bytes for the previous frame pointer and 8 bytes for the return
+// address both stored on the stack after at the beginning of the current
+// frame.
+#  define REGISTERS_SYNC_POPULATE(regs)                                       \
+    regs.mSP = reinterpret_cast<Address>(__builtin_frame_address(0)) + 0x10;  \
+    _Pragma("GCC diagnostic push")                                            \
+    _Pragma("GCC diagnostic ignored \"-Wframe-address\"")                     \
+    regs.mFP = reinterpret_cast<Address>(__builtin_frame_address(1));         \
+    _Pragma("GCC diagnostic pop")                                             \
+    regs.mPC = reinterpret_cast<Address>(                                     \
+        __builtin_extract_return_addr(__builtin_return_address(0)));          \
+    regs.mLR = 0;
 #endif
+// clang-format on
 
 }  // namespace baseprofiler
 }  // namespace mozilla

mozglue/baseprofiler/core/platform-win32.cpp

@@ -288,11 +288,10 @@ void SamplerThread::Stop(PSLockRef aLock) {
 static void PlatformInit(PSLockRef aLock) {}
 
 #if defined(HAVE_NATIVE_UNWIND)
-void Registers::SyncPopulate() {
-  CONTEXT context;
-  RtlCaptureContext(&context);
-  PopulateRegsFromContext(*this, &context);
-}
+#  define REGISTERS_SYNC_POPULATE(regs) \
+    CONTEXT context;                    \
+    RtlCaptureContext(&context);        \
+    PopulateRegsFromContext(regs, &context);
 #endif
 
 #if defined(GP_PLAT_amd64_windows)

mozglue/baseprofiler/core/platform.cpp

@@ -17,7 +17,7 @@
 //   call (profiler_get_backtrace()). It involves writing a stack trace and
 //   little else into a temporary ProfileBuffer, and wrapping that up in a
 //   ProfilerBacktrace that can be subsequently used in a marker. The sampling
-//   is done on-thread, and so Registers::SyncPopulate() is used to get the
+//   is done on-thread, and so REGISTERS_SYNC_POPULATE() is used to get the
 //   register values.
 //
 // - A "backtrace" sample is the simplest kind. It is done in response to an
@@ -105,7 +105,7 @@
 // No stack-walking in baseprofiler on linux, android, bsd.
 // APIs now make it easier to capture backtraces from the Base Profiler, which
 // is currently not supported on these platform, and would lead to a MOZ_CRASH
-// in Registers::SyncPopulate(). `#if 0` added in bug 1658232, follow-up bugs
+// in REGISTERS_SYNC_POPULATE(). `#if 0` added in bug 1658232, follow-up bugs
 // should be referenced in meta bug 1557568.
 #if 0
 // Android builds use the ARM Exception Handling ABI to unwind.
@@ -1246,16 +1246,11 @@ class Registers {
  public:
   Registers() : mPC{nullptr}, mSP{nullptr}, mFP{nullptr}, mLR{nullptr} {}
 
-#if defined(HAVE_NATIVE_UNWIND)
-  // Fills in mPC, mSP, mFP, mLR, and mContext for a synchronous sample.
-  void SyncPopulate();
-#endif
-
   void Clear() { memset(this, 0, sizeof(*this)); }
 
   // These fields are filled in by
   // Sampler::SuspendAndSampleAndResumeThread() for periodic and backtrace
-  // samples, and by SyncPopulate() for synchronous samples.
+  // samples, and by REGISTERS_SYNC_POPULATE for synchronous samples.
   Address mPC;  // Instruction pointer.
   Address mSP;  // Stack pointer.
   Address mFP;  // Frame pointer.
@@ -3667,7 +3662,7 @@ bool profiler_capture_backtrace_into(ProfileChunkedBuffer& aChunkedBuffer,
 
   Registers regs;
 #if defined(HAVE_NATIVE_UNWIND)
-  regs.SyncPopulate();
+  REGISTERS_SYNC_POPULATE(regs);
 #else
   regs.Clear();
 #endif
@@ -3801,7 +3796,7 @@ void profiler_suspend_and_sample_thread(BaseProfilerThreadId aThreadId,
         // Sampling the current thread, do NOT suspend it!
         Registers regs;
 #if defined(HAVE_NATIVE_UNWIND)
-        regs.SyncPopulate();
+        REGISTERS_SYNC_POPULATE(regs);
 #else
         regs.Clear();
 #endif

tools/profiler/core/platform-linux-android.cpp

@@ -629,9 +629,8 @@ static void PlatformInit(PSLockRef aLock) {}
 #endif
 
 #if defined(HAVE_NATIVE_UNWIND)
-void Registers::SyncPopulate() {
-  if (!getcontext(&mContextSyncStorage)) {
-    PopulateRegsFromContext(*this, &mContextSyncStorage);
-  }
-}
+#  define REGISTERS_SYNC_POPULATE(regs)                         \
+    if (!getcontext(&regs.mContextSyncStorage)) {               \
+      PopulateRegsFromContext(regs, &regs.mContextSyncStorage); \
+    }
 #endif

tools/profiler/core/platform-macos.cpp

@@ -278,19 +278,20 @@ void SamplerThread::Stop(PSLockRef aLock) { mSampler.Disable(aLock); }
 
 static void PlatformInit(PSLockRef aLock) {}
 
+// clang-format off
 #if defined(HAVE_NATIVE_UNWIND)
-void Registers::SyncPopulate() {
-  // Derive the stack pointer from the frame pointer. The 0x10 offset is
-  // 8 bytes for the previous frame pointer and 8 bytes for the return
-  // address both stored on the stack after at the beginning of the current
-  // frame.
-  mSP = reinterpret_cast<Address>(__builtin_frame_address(0)) + 0x10;
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wframe-address"
-  mFP = reinterpret_cast<Address>(__builtin_frame_address(1));
-#  pragma GCC diagnostic pop
-  mPC = reinterpret_cast<Address>(
-      __builtin_extract_return_addr(__builtin_return_address(0)));
-  mLR = 0;
-}
+// Derive the stack pointer from the frame pointer. The 0x10 offset is
+// 8 bytes for the previous frame pointer and 8 bytes for the return
+// address both stored on the stack after at the beginning of the current
+// frame.
+#  define REGISTERS_SYNC_POPULATE(regs)                                       \
+    regs.mSP = reinterpret_cast<Address>(__builtin_frame_address(0)) + 0x10;  \
+    _Pragma("GCC diagnostic push")                                            \
+    _Pragma("GCC diagnostic ignored \"-Wframe-address\"")                     \
+    regs.mFP = reinterpret_cast<Address>(__builtin_frame_address(1));         \
+    _Pragma("GCC diagnostic pop")                                             \
+    regs.mPC = reinterpret_cast<Address>(                                     \
+        __builtin_extract_return_addr(__builtin_return_address(0)));          \
+    regs.mLR = 0;
 #endif
+// clang-format on

tools/profiler/core/platform-win32.cpp

@@ -475,11 +475,10 @@ void SamplerThread::Stop(PSLockRef aLock) {
 static void PlatformInit(PSLockRef aLock) {}
 
 #if defined(HAVE_NATIVE_UNWIND)
-void Registers::SyncPopulate() {
-  CONTEXT context;
-  RtlCaptureContext(&context);
-  PopulateRegsFromContext(*this, &context);
-}
+#  define REGISTERS_SYNC_POPULATE(regs) \
+    CONTEXT context;                    \
+    RtlCaptureContext(&context);        \
+    PopulateRegsFromContext(regs, &context);
 #endif
 
 #if defined(GP_PLAT_amd64_windows)

tools/profiler/core/platform.cpp

@@ -17,7 +17,7 @@
 //   call (profiler_get_backtrace()). It involves writing a stack trace and
 //   little else into a temporary ProfileBuffer, and wrapping that up in a
 //   ProfilerBacktrace that can be subsequently used in a marker. The sampling
-//   is done on-thread, and so Registers::SyncPopulate() is used to get the
+//   is done on-thread, and so REGISTERS_SYNC_POPULATE() is used to get the
 //   register values.
 //
 // - A "backtrace" sample is the simplest kind. It is done in response to an
@@ -1637,16 +1637,11 @@ class Registers {
  public:
   Registers() : mPC{nullptr}, mSP{nullptr}, mFP{nullptr}, mLR{nullptr} {}
 
-#if defined(HAVE_NATIVE_UNWIND)
-  // Fills in mPC, mSP, mFP, mLR, and mContext for a synchronous sample.
-  void SyncPopulate();
-#endif
-
   void Clear() { memset(this, 0, sizeof(*this)); }
 
   // These fields are filled in by
   // Sampler::SuspendAndSampleAndResumeThread() for periodic and backtrace
-  // samples, and by SyncPopulate() for synchronous samples.
+  // samples, and by REGISTERS_SYNC_POPULATE for synchronous samples.
   Address mPC;  // Instruction pointer.
   Address mSP;  // Stack pointer.
   Address mFP;  // Frame pointer.
@@ -6774,7 +6769,7 @@ bool profiler_capture_backtrace_into(ProfileChunkedBuffer& aChunkedBuffer,
 
         Registers regs;
 #if defined(HAVE_NATIVE_UNWIND)
-        regs.SyncPopulate();
+        REGISTERS_SYNC_POPULATE(regs);
 #else
         regs.Clear();
 #endif
@@ -6978,7 +6973,7 @@ static void profiler_suspend_and_sample_thread(
     // Sampling the current thread, do NOT suspend it!
     Registers regs;
 #if defined(HAVE_NATIVE_UNWIND)
-    regs.SyncPopulate();
+    REGISTERS_SYNC_POPULATE(regs);
 #else
     regs.Clear();
 #endif