Closed Bug 1549232 Opened 2 years ago Closed 2 years ago

--enable-tasktracer --enable-warnings-as-errors -> GeckoTaskTracer.cpp(27,11): error: 'getpid' macro redefined


(Core :: Gecko Profiler, defect, P3)




Tracking Status
firefox68 --- fixed


(Reporter: gerald, Assigned: gerald)



(1 file)

With ac_add_options --enable-tasktracer and ac_add_options --enable-warnings-as-errors in mozconfig, I get the following build failure on Windows:

In file included from c:/mozilla-source/obj-mc-dbg/tools/profiler/Unified_cpp_tools_profiler1.cpp:11:
  error: 'getpid' macro redefined [-Werror,-Wmacro-redefined]
#  define getpid GetCurrentProcessId
  note: previous definition is here
#    define getpid _getpid
1 error generated.
mozmake.EXE[4]: *** [c:/mozilla-source/mozilla-central/config/
  Unified_cpp_tools_profiler1.obj] Error 1

Instead of having these ad-hoc getpid kludges in multiple files, I would suggest implementing platform-specific functions in platform-x.cpp, with a public profiler_current_process_id() API in GeckoProfiler.h. This is consistent with the existing profiler_current_thread_id() (though it is not consistently used everywhere, easy to fix as well).

This will be extra useful when porting to mozglue.

I think I've accidentally regressed this when adding a #include "platform.h" in bug 1520103.

Keywords: regression
Regressed by: 1520103

Bug 1520103 only moved the #include "platform.h", so that's not it.

Keywords: regression
No longer regressed by: 1520103

There were many inconsistent ways to retrieve process/thread ids in the
profiler, so now have only one implementation each (per platform) :
profiler_current_process_id() and profiler_current_thread_id().

Note that this removes the need for the small class Thread in platform.h.
However memory_hooks.cpp still need to be built non-unified, because of the
required order of #includes (replace_malloc.h before replace_malloc_bridge.h),
which could be disturbed by other cpp's.

Attachment #9062829 - Attachment description: Bug 1549232 - Profiler only uses profiler_current_{process,thread}_id instead of alternatives - r?mstange → Bug 1549232 - Only use profiler_current_{process,thread}_id in the Gecko Profiler instead of alternatives - r?mstange

Try with --enable-tasktracer, to test that we're fixing the original build issue:

Pushed by
Only use profiler_current_{process,thread}_id in the Gecko Profiler instead of alternatives - r=mstange
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.