Windows build failure - xul.dll fails to link due to: error LNK2019: unresolved external symbol "__declspec(dllimport) bool __cdecl MozStackWalk

RESOLVED FIXED in Firefox 53

Status

()

defect
P1
blocker
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wgianopoulos, Assigned: Dexter)

Tracking

(Blocks 1 bug, {regression})

53 Branch
mozilla53
x86_64
Windows
Points:
1
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

(Whiteboard: [measurement:client])

Attachments

(1 attachment, 2 obsolete attachments)

Here are the logfile errors:

 6:57.17 Telemetry.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) bool __cdecl MozStackWalk(void (__cdecl*)(unsigned int,void *,void *,void *),unsigned int,unsigned int,void *,unsigned int,void *)" (__imp_?MozStackWalk@@YA_NP6AXIPAX00@ZII0I0@Z) referenced in function "public: void __thiscall `anonymous namespace'::KeyedStackCapturer::Capture(class nsACString_internal const &)" (?Capture@KeyedStackCapturer@?A0x07cb84f2@@QAEXABVnsACString_internal@@@Z)
 6:57.17
 6:57.17 xul.dll : fatal error LNK1120: 1 unresolved externals
and here is my .mozconfig

. $topsrcdir/browser/config/mozconfig
BUILD_OFFICIAL=1
export BUILD_OFFICIAL
WIN32_REDIST_DIR=~/mozilla/redist/x86
export WIN32_REDIST_DIR
#WIN32_CRT_SRC_DIR=~/mozilla/crtsrc
mk_add_options BUILD_OFFICIAL=1
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/fx-obj
mk_add_options RUN_AUTOCONF_LOCALLY=1
mk_add_options AUTOCONF=/usr/local/bin/autoconf-2.13
ac_add_options --enable-optimize=-O2
ac_add_options --disable-debug
ac_add_options --enable-tests
ac_add_options --disable-updater
ac_add_options --disable-crashreporter
ac_add_options --enable-jemalloc
ac_add_options --enable-eme=adobe
ac_add_options --enable-require-all-d3dc-versions
mk_add_options MOZ_MAKE_FLAGS="-j3"
I suspect bug 1225851.  I am trying just rebuilding in case this was just a hardware issue on my side.  If that fails, I will try backing 1225851 out.
Keywords: regression
I tried the back out of bug 1225851 and it worked then.
Blocks: 1225851
Component: Build Config → Telemetry
Product: Core → Toolkit
Summary: Can no longer build windows trunk → Can no longer build windows trunk since bug 1225851 patches landed.
Bill, do you have tests disabled in your mozconfig? Maybe this is the reason why the mozilla builder work.
Ah I did post my mozconfig in comment #1.  I do not have tests disabled, however I do have crashreporter disabled.
OH and also updater because i do not support automatic updates of my builds.
Mh, I think the MozStackWalk call in Telemetry.cpp [1] needs to be conditionally enabled/disabled based on the MOZ_STACKWALKING define [2], or at least that's what the comment says.

[1] - https://dxr.mozilla.org/mozilla-central/rev/bad312aefb42982f492ad2cf36f4c6c3d698f4f7/toolkit/components/telemetry/Telemetry.cpp#519
[2] - https://dxr.mozilla.org/mozilla-central/rev/bad312aefb42982f492ad2cf36f4c6c3d698f4f7/mozglue/misc/StackWalk.h#56
(In reply to Alessio Placitelli [:Dexter] from comment #7)
> Mh, I think the MozStackWalk call in Telemetry.cpp [1] needs to be
> conditionally enabled/disabled based on the MOZ_STACKWALKING define [2], or
> at least that's what the comment says.
> 
> [1] -
> https://dxr.mozilla.org/mozilla-central/rev/
> bad312aefb42982f492ad2cf36f4c6c3d698f4f7/toolkit/components/telemetry/
> Telemetry.cpp#519
> [2] -
> https://dxr.mozilla.org/mozilla-central/rev/
> bad312aefb42982f492ad2cf36f4c6c3d698f4f7/mozglue/misc/StackWalk.h#56

Alternatively this code needs to include what is required to make the stackwalk work and no be dependent on having crashreporter enabled.
Georg, ni? you so it's on our radar for the next week.
Points: --- → 1
Flags: needinfo?(gfritzsche)
Priority: -- → P1
Whiteboard: [measurement:client]
It would seem to me that the correct fix is to have MOZ-STACKWALKING defined if either crashreporter or telemetry is enabled for the build.
ah so it really seems you need to have enable-profiling for this to work.  I can do that.
Duplicate of this bug: 1320661
Summary: Can no longer build windows trunk since bug 1225851 patches landed. → Windows build failure - xul.dll fails to link due to: error LNK2019: unresolved external symbol "__declspec(dllimport) bool __cdecl MozStackWalk
Assignee: nobody → gfritzsche
Flags: needinfo?(gfritzsche)
Stealing the bug!
Assignee: gfritzsche → alessio.placitelli
Version: Trunk → 53 Branch
Is there a way for Windows developers to build while this is being fixed? I updated my tree and now I cannot build.
(In reply to Kirk Steuber [:bytesized] from comment #14)
> Is there a way for Windows developers to build while this is being fixed? I
> updated my tree and now I cannot build.

Adding

ac_add_options --enable-profiling

to your mozconfig would be the simplest solution.
Building on Windows without a mozconfig is currently failing due to this bug. This state in unacceptable because it is too disruptive.

Please either land a fix for this bug ASAP or revert 620b85825ca5.
Flags: needinfo?(alessio.placitelli)
Posted patch bug1320312.patch (obsolete) — Splinter Review
The patch does this:

- Changes Telemetry.cpp to disable stack walking if MOZ_STACKWALKING is not defined.
- Adds MOZ_STACKWALKING to AppConstant so that it can be used to disable tests.
- Changes the stack capturing tests to be skipped if AppConstant.MOZ_STACKWALKING is not defined.
Flags: needinfo?(alessio.placitelli)
Attachment #8815175 - Flags: review?(gfritzsche)
Status: NEW → ASSIGNED
(In reply to Gregory Szorc [:gps] from comment #16)
> Building on Windows without a mozconfig is currently failing due to this
> bug. This state in unacceptable because it is too disruptive.

If this is an important build configuration, maybe this should get CI coverage?
Flags: needinfo?(gps)
Thanks for fixing the code :Dexter!
Comment on attachment 8815175 [details] [diff] [review]
bug1320312.patch

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

::: toolkit/components/telemetry/Telemetry.cpp
@@ -81,3 @@
>  
>  namespace {
> -

Unrelated change, please undo.
There are a few more of discarded empty lines below in this patch here.

@@ +395,5 @@
>  const nsClassHashtable<nsStringHashKey, HangReports::AnnotationInfo>&
>  HangReports::GetAnnotationInfo() const {
>    return mAnnotationInfo;
>  }
> +#if defined(ENABLE_STACK_CAPTURE)

Add an empty line before & after this one.

@@ -922,5 @@
>    bool GetSQLStats(JSContext *cx, JS::MutableHandle<JS::Value> ret,
>                     bool includePrivateSql);
>  
>    void ReadLateWritesStacks(nsIFile* aProfileDir);
> -

Unrelated change, please undo.

@@ +929,5 @@
>    AutoHashtable<SlowSQLEntryType> mSanitizedSQL;
>    Mutex mHashMutex;
>    HangReports mHangReports;
>    Mutex mHangReportsMutex;
> +#if defined(ENABLE_STACK_CAPTURE)

Please undo the removal of the empty lines here & below.

@@ +3141,5 @@
>                                    Move(aAnnotations));
>  }
> +#endif
> +
> +#if defined(ENABLE_STACK_CAPTURE)

This needs to behave the same as the declaration in Telemetry.h
The intention here was that Telemetry::CaptureStack() is always declared, but internally it's a no-op when ENABLE_STACK_CAPTURE is not defined (in TelemetryImpl::CaptureStack()).

It looks like we didn't cover this too well in the review before.
Attachment #8815175 - Flags: review?(gfritzsche)
Posted patch bug1320312.patch (obsolete) — Splinter Review
Whoops, sorry Georg, the discarded blank lines were unintended.
Attachment #8815175 - Attachment is obsolete: true
Attachment #8815255 - Flags: review?(gfritzsche)
Comment on attachment 8815255 [details] [diff] [review]
bug1320312.patch

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

Thanks!
Attachment #8815255 - Flags: review?(gfritzsche) → review+
+#if defined(MOZ_STACKWALKING)
+#define ENABLE_STACK_CAPTURE
 #include "mozilla/StackWalk.h"
 #include "nsPrintfCString.h"
-#endif
+#endif // ENABLE_STACK_CAPTURE
           ^^^^^^^^^^^^^^^^^^^^

Shouldn't this comment say MOZ_STACKWALKING to match the condition on the #if?
(In reply to Bill Gianopoulos [:WG9s] from comment #24)
> +#if defined(MOZ_STACKWALKING)
> +#define ENABLE_STACK_CAPTURE
>  #include "mozilla/StackWalk.h"
>  #include "nsPrintfCString.h"
> -#endif
> +#endif // ENABLE_STACK_CAPTURE
>            ^^^^^^^^^^^^^^^^^^^^
> 
> Shouldn't this comment say MOZ_STACKWALKING to match the condition on the
> #if?

Yup, good catch.
Attachment #8815255 - Attachment is obsolete: true
Attachment #8815311 - Flags: review+
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15d6ef5ef0e2
Disable Telemetry stack capturing if stack walking is not available. r=gfritzsche
https://hg.mozilla.org/integration/mozilla-inbound/rev/15d6ef5ef0e2dfba731d4045813bf0f3828eb5b8
Bug 1320312 - Disable Telemetry stack capturing if stack walking is not available. r=gfritzsche
(In reply to Georg Fritzsche [:gfritzsche] from comment #18)
> (In reply to Gregory Szorc [:gps] from comment #16)
> > Building on Windows without a mozconfig is currently failing due to this
> > bug. This state in unacceptable because it is too disruptive.
> 
> If this is an important build configuration, maybe this should get CI
> coverage?

This is a very valid point and I will follow-up on it in another bug.

While I'm here, nobody involved with this breakage should feel bad about breaking something that CI didn't catch: the fault is with CI for not testing build configurations that developers rely on.
Flags: needinfo?(gps)
Blocks: 1321041
Blocks: 1321065
https://hg.mozilla.org/mozilla-central/rev/15d6ef5ef0e2
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.