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

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: gerald, Assigned: gerald)

Tracking

Trunk
mozilla68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

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.

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:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e09f59dd98fbbb7f9903e55405c5bf9b99e1a541

Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ddac071d565
Only use profiler_current_{process,thread}_id in the Gecko Profiler instead of alternatives - r=mstange
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.