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

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P3)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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:
c:/mozilla-source/mozilla-central/tools/profiler/tasktracer/GeckoTaskTracer.cpp(27,11):
  error: 'getpid' macro redefined [-Werror,-Wmacro-redefined]
#  define getpid GetCurrentProcessId
          ^
c:/mozilla-source/mozilla-central/tools/profiler/core\platform.h(58,13):
  note: previous definition is here
#    define getpid _getpid
            ^
1 error generated.
mozmake.EXE[4]: *** [c:/mozilla-source/mozilla-central/config/rules.mk:805:
  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.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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

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.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try with --enable-tasktracer, to test that we're fixing the original build issue:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e09f59dd98fbbb7f9903e55405c5bf9b99e1a541

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ddac071d565
Only use profiler_current_{process,thread}_id in the Gecko Profiler instead of alternatives - r=mstange

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3ddac071d565