1145988 - dist/include/GeckoProfiler.h:239:5: error: use of undeclared identifier 'MOZ_GUARD_OBJECT_NOTIFIER_INIT'

Assignee

Description

•

9 years ago

On platforms lacking SPS profiler (e.g. Linux/PPC, BSDs, Solaris) the build is broken likely due to header bootlegging. Injecting #error into mfbt/GuardObjects.h may explain how the header ends up being included there with SPS.

  In file included from netwerk/base/Unified_cpp_netwerk_base2.cpp:2:
  In file included from netwerk/base/nsInputStreamPump.cpp:8:
  In file included from netwerk/base/nsInputStreamPump.h:14:
  In file included from dist/include/mozilla/ReentrantMonitor.h:13:
  dist/include/GeckoProfiler.h:235:28: error: expected ')'
                             MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
                             ^
  dist/include/GeckoProfiler.h:233:27: note: to match this '('
    GeckoProfilerTracingRAII(const char* aCategory, const char* aInfo,
                            ^
  dist/include/GeckoProfiler.h:248:3: error: unknown type name
        'MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER'
    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
    ^
  dist/include/GeckoProfiler.h:249:3: error: expected member name or ';' after
        declaration specifiers
    const char* mCategory;
    ^
  dist/include/GeckoProfiler.h:236:7: error: member initializer 'mCategory' does
        not name a non-static data member or base class
      : mCategory(aCategory)
        ^~~~~~~~~~~~~~~~~~~~
  dist/include/GeckoProfiler.h:239:5: error: use of undeclared identifier
        'MOZ_GUARD_OBJECT_NOTIFIER_INIT'
      MOZ_GUARD_OBJECT_NOTIFIER_INIT;
      ^
  dist/include/GeckoProfiler.h:240:22: error: use of undeclared identifier
        'mCategory'; did you mean 'aCategory'?
      profiler_tracing(mCategory, mInfo, aBacktrace.release(), TRACING_INTERVA...
                       ^~~~~~~~~
                       aCategory
  dist/include/GeckoProfiler.h:233:40: note: 'aCategory' declared here
    GeckoProfilerTracingRAII(const char* aCategory, const char* aInfo,
                                         ^
  dist/include/GeckoProfiler.h:244:22: error: use of undeclared identifier
        'mCategory'
      profiler_tracing(mCategory, mInfo, TRACING_INTERVAL_END);
                       ^
  7 errors generated.

and only GeckoProfiler.h under tools/profiler/ is exported without SPS

  In file included from layout/generic/Unified_cpp_layout_generic0.cpp:47:
  In file included from layout/generic/StickyScrollContainer.cpp:16:
  layout/generic/../base/RestyleTracker.h:19:10: fatal error:
        'ProfilerBacktrace.h' file not found
  #include "ProfilerBacktrace.h"
           ^
  1 error generated.

but the header is required to avoid the following

  dist/include/mozilla/UniquePtr.h:488:19: error: invalid application of
        'sizeof' to an incomplete type 'ProfilerBacktrace'
      static_assert(sizeof(T) > 0, "T must be complete");
                    ^~~~~~~~~
  dist/include/mozilla/UniquePtr.h:308:7: note: in instantiation of member
        function 'mozilla::DefaultDelete<ProfilerBacktrace>::operator()' requested
        here
        getDeleter()(old);
        ^
  layout/generic/../base/RestyleTracker.h:399:22: note:
        in instantiation of member function 'mozilla::UniquePtr<ProfilerBacktrace,
        mozilla::DefaultDelete<ProfilerBacktrace> >::reset' requested here
        rd->mBacktrace.reset(profiler_get_backtrace());
                       ^
  dist/include/GeckoProfiler.h:101:7: note: forward declaration of
        'ProfilerBacktrace'
  class ProfilerBacktrace;
        ^
  1 error generated.

Jan Beich

Assignee

Comment 1

•

9 years ago

Attached patch fix, v0 (obsolete) — Details — Splinter Review

Not sure how much to comment out or provide a stub for |class ProfilerBacktrace|.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fdc359be5655

Attachment #8581152 - Flags: feedback?

Jan Beich

Assignee

Updated

•

9 years ago

Attachment #8581152 - Flags: feedback? → feedback?(chung)

Chiajung Hung [:chiajung]

Comment 2

•

9 years ago

Comment on attachment 8581152 [details] [diff] [review]
fix, v0

Review of attachment 8581152 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Attachment #8581152 - Flags: feedback?(chung) → feedback+

Landry Breuil (:gaston)

Comment 4

•

9 years ago

Comment on attachment 8581152 [details] [diff] [review]
fix, v0

Adding a different reviewer so that this hopefully lands..

Attachment #8581152 - Flags: review?(dholbert)

Daniel Holbert [:dholbert]

Comment 5

•

9 years ago

Comment on attachment 8581152 [details] [diff] [review]
fix, v0

Review of attachment 8581152 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/RestyleTracker.h
@@ +19,5 @@
>  #include "GeckoProfiler.h"
>  
> +#ifdef MOZ_ENABLE_PROFILER_SPS
> +#include "ProfilerBacktrace.h"
> +#endif

Can this just be a forward-declaration of "class ProfilerBacktrace"?

(I think it can? If so, I wouldn't bother with ifdefs around the forward-decl; and the #include can probably move to RestyleTracker.cpp, if we even need it there (we probably do).)

@@ +402,1 @@
>        rd->mBacktrace.reset(profiler_get_backtrace());

The #ifdef/#endif should move out one curly-brace level -- it should be outside the profiler_feature_active() call.

Daniel Holbert [:dholbert]

Comment 6

•

9 years ago

Comment on attachment 8581152 [details] [diff] [review]
fix, v0

(In reply to Daniel Holbert [:dholbert] from comment #5)
> (I think it can? If so, I wouldn't bother with ifdefs around the
> forward-decl; and the #include can probably move to RestyleTracker.cpp, if
> we even need it there (we probably do).)

Locally, I can build with just the forward-decl -- no need to move the #include to RestyleTracker.cpp -- which makes sense because I think we just pass & receive ProfilerBacktrace pointers to & from profiler functions. We never actually do anything directly with the ProfilerBacktrace.

I'm willing to believe that I'm just getting saved by unified builds and/or some other attribute of my build environment, though.

r+ with both parts of comment 5 addressed (the first part ideally with just a forward-dec; if you need the #include, r=me still stands, but I'm curious where it ends up being necessary.)

Attachment #8581152 - Flags: review?(dholbert) → review+

Jan Beich

Assignee

Comment 7

•

9 years ago

Attached patch fix, v1 — Details — Splinter Review

(In reply to Daniel Holbert [:dholbert] from comment #5)
> > +#ifdef MOZ_ENABLE_PROFILER_SPS
> > +#include "ProfilerBacktrace.h"
> > +#endif
> 
> Can this just be a forward-declaration of "class ProfilerBacktrace"?

Let me quote the last error in comment 0:

  dist/include/mozilla/UniquePtr.h:488:19: error: invalid application of
        'sizeof' to an incomplete type 'ProfilerBacktrace'
      static_assert(sizeof(T) > 0, "T must be complete");
                    ^~~~~~~~~
  dist/include/mozilla/UniquePtr.h:308:7: note: in instantiation of member
        function 'mozilla::DefaultDelete<ProfilerBacktrace>::operator()' requested
        here
        getDeleter()(old);
        ^
  layout/generic/../base/RestyleTracker.h:399:22: note:
        in instantiation of member function 'mozilla::UniquePtr<ProfilerBacktrace,
        mozilla::DefaultDelete<ProfilerBacktrace> >::reset' requested here
        rd->mBacktrace.reset(profiler_get_backtrace());
                       ^
  dist/include/GeckoProfiler.h:101:7: note: forward declaration of
        'ProfilerBacktrace'
  class ProfilerBacktrace;
        ^
  1 error generated.

> (I think it can? If so, I wouldn't bother with ifdefs around the
> forward-decl; and the #include can probably move to RestyleTracker.cpp, if
> we even need it there (we probably do).)

In comment 1 by stub I meant to make smth like the following generic
and put under |#ifndef MOZ_ENABLE_PROFILER_SPS| in GeckoProfiler.h.

  #ifdef MOZ_ENABLE_PROFILER_SPS
      UniquePtr<ProfilerBacktrace> mBacktrace;
  #else
      UniquePtr<NULL> mBacktrace;
  #endif

> @@ +402,1 @@
> >        rd->mBacktrace.reset(profiler_get_backtrace());
> 
> The #ifdef/#endif should move out one curly-brace level -- it should be
> outside the profiler_feature_active() call.

Done in v1: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3db5d66a9e65

(In reply to Daniel Holbert [:dholbert] from comment #6)
> Locally, I can build with just the forward-decl -- no need to move the
> #include to RestyleTracker.cpp -- which makes sense because I think we just
> pass & receive ProfilerBacktrace pointers to & from profiler functions. We
> never actually do anything directly with the ProfilerBacktrace.

Please, do a proper non-SPS build by modifying configure.in.
SPS bootlegs ProfilerBacktrace.h via GeckoProfiler.h.

Here's an example:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e47266fc83a3

  ../../dist/include/mozilla/UniquePtr.h:489:16: error: invalid application of 'sizeof' to incomplete type 'ProfilerBacktrace'
  ../../dist/include/mozilla/UniquePtr.h:489:5: error: possible problem detected in invocation of delete operator: [-Werror]
  ../../dist/include/mozilla/UniquePtr.h:486:8: error: 'aPtr' has incomplete type [-Werror]
  ../../dist/include/GeckoProfiler.h:101:7: error: forward declaration of 'class ProfilerBacktrace' [-Werror]
  gmake[5]: *** [Unified_cpp_layout_generic0.o] Error 1
  gmake[5]: *** Waiting for unfinished jobs....
  ../../dist/include/mozilla/UniquePtr.h:489:16: error: invalid application of 'sizeof' to incomplete type 'ProfilerBacktrace'
  ../../dist/include/mozilla/UniquePtr.h:489:5: error: possible problem detected in invocation of delete operator: [-Werror]
  ../../dist/include/mozilla/UniquePtr.h:486:8: error: 'aPtr' has incomplete type [-Werror]
  ../../dist/include/GeckoProfiler.h:101:7: error: forward declaration of 'class ProfilerBacktrace' [-Werror]
  gmake[5]: *** [Unified_cpp_layout_generic1.o] Error 1
  ../../dist/include/mozilla/UniquePtr.h:489:16: error: invalid application of 'sizeof' to incomplete type 'ProfilerBacktrace'
  ../../dist/include/mozilla/UniquePtr.h:489:5: error: possible problem detected in invocation of delete operator: [-Werror]
  ../../dist/include/mozilla/UniquePtr.h:486:8: error: 'aPtr' has incomplete type [-Werror]
  ../../dist/include/GeckoProfiler.h:101:7: error: forward declaration of 'class ProfilerBacktrace' [-Werror]
  gmake[5]: *** [Unified_cpp_layout_generic2.o] Error 1
  ../../dist/include/mozilla/UniquePtr.h:489:16: error: invalid application of 'sizeof' to incomplete type 'ProfilerBacktrace'
  ../../dist/include/mozilla/UniquePtr.h:489:5: error: possible problem detected in invocation of delete operator: [-Werror]
  ../../dist/include/mozilla/UniquePtr.h:486:8: error: 'aPtr' has incomplete type [-Werror]
  ../../dist/include/GeckoProfiler.h:101:7: error: forward declaration of 'class ProfilerBacktrace' [-Werror]
  gmake[5]: *** [Unified_cpp_layout_tables0.o] Error 1

Attachment #8581152 - Attachment is obsolete: true

Attachment #8586863 - Flags: review?(dholbert)

Daniel Holbert [:dholbert]

Comment 8

•

9 years ago

Comment on attachment 8586863 [details] [diff] [review]
fix, v1

Gotcha. I hadn't realized UniquePtr<Foo> requires Foo to be defined. (nsRefPtr / nsAutoPtr do not have that requirement, IIRC)

Thanks for catching & fixing the bustage!

r=me

Attachment #8586863 - Flags: review?(dholbert) → review+

Landry Breuil (:gaston)

Comment 9

•

9 years ago

m-i's closed, setting checkin-needed...

Keywords: checkin-needed

Landry Breuil (:gaston)

Comment 10

•

9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4de1ed6e851d

Landry Breuil (:gaston)

Updated

•

9 years ago

Keywords: checkin-needed

Ryan VanderMeulen [:RyanVM]

Comment 11

•

9 years ago

https://hg.mozilla.org/mozilla-central/rev/4de1ed6e851d

Status: NEW → RESOLVED

Closed: 9 years ago

status-firefox40: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla40

Jan Beich

Assignee

Comment 12

•

9 years ago

Comment on attachment 8586863 [details] [diff] [review]
fix, v1

Approval Request Comment
[Feature/regressing bug #]: bug 1129249 regression
[User impact if declined]: broken build on non-SPS platforms
[Describe test coverage new/current, TreeHerder]: m-c, m-i, Try in comment 7
[Risks and why]: Low: due to typos either broken build or new code from bug 1129249 being accidentally disabled for SPS platforms
[String/UUID change made/needed]: None

Attachment #8586863 - Flags: approval-mozilla-aurora?

Landry Breuil (:gaston)

Updated

•

9 years ago

status-firefox38: --- → unaffected

status-firefox39: --- → affected

Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)

Comment 13

•

9 years ago

Comment on attachment 8586863 [details] [diff] [review]
fix, v1

Approving this for 39 since it's a recent regression and looks like a low-risk fix.

Attachment #8586863 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Ryan VanderMeulen [:RyanVM]

Comment 14

•

9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a6b36e4eac9

status-firefox39: affected → fixed

Jan Beich

Assignee

Updated

•

9 years ago

Blocks: 1151829

Jan Beich

Assignee

Updated

•

9 years ago

Assignee: nobody → jbeich

fix, v0 9 years ago Jan Beich 5.52 KB, patch	dholbert : review+ chiajung : feedback+	Details \| Diff \| Splinter Review
fix, v1 9 years ago Jan Beich 5.71 KB, patch	dholbert : review+ lizzard : approval-mozilla-aurora+	Details \| Diff \| Splinter Review

Bugzilla

Quick Search

dist/include/GeckoProfiler.h:239:5: error: use of undeclared identifier 'MOZ_GUARD_OBJECT_NOTIFIER_INIT'

Categories

(Core :: Gecko Profiler, defect)

Tracking

()

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

Crash Data

Security

(public)

User Story

Attachments

(1 file, 1 obsolete file)

Description

Comment 1

Updated

Comment 2

Comment 4

Comment 5

Comment 6

Comment 7

Comment 8

Comment 9

Comment 10

Updated

Comment 11

Comment 12

Updated

Comment 13

Comment 14

Updated

Updated