Closed Bug 1244006 Opened 4 years ago Closed 4 years ago

startup crash after bug 1230034

Categories

(Core :: Layout, defect)

x86_64
Windows 10
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: c, Assigned: gps)

References

Details

Attachments

(1 file)

After bug 1230034 lands, vc14 wpo(-GL) builds crash at startup
Is there any message when crash happens?

How should I enable wpo? (Probably via enabling PGO?)
Flags: needinfo?(zhoubcfan)
Also, what version of compiler are you using?
no message.
easy way: compile layout/generic with -GL (I'm not sure because there are other flags)
cl 19.00.23506 sdk 10.0.10586
Flags: needinfo?(zhoubcfan)
According to the document in Visual Studio, Whole Program Optimization requires the linker option 'Link Time Code Generation' be turned on. Do I need to change anything else to reproduce it given this requirement?
Flags: needinfo?(zhoubcfan)
using -GL will implicitly turning on ltcg
Flags: needinfo?(zhoubcfan)
By the way, I have OS_LDFLAGS += -DEBUG -OPT:REF,ICF (http://mxr.mozilla.org/mozilla-central/source/config/config.mk#206) unconditional set(I move it out of the ifdef)
Hmmm, adding `CPPFLAGS += ['-GL']` to layout/generic/moz.build doesn't seem to reproduce this issue.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #7)
> Hmmm, adding `CPPFLAGS += ['-GL']` to layout/generic/moz.build doesn't seem
> to reproduce this issue.

CXXFLAGS
Yeah... I meant, CXXFLAGS. Also I saw
> nsLineLayout.obj : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance
in the output, so it has already been working anyway.

Probably need to be in a wider range.
Here are the command lines

F:/develop/mozilla/central/obj/obj-VC64/_virtualenv/Scripts/python.exe -m mozbuild.action.cl  cl -FonsLineLayout.obj -c -IF:/develop/mozilla/central/obj/obj-VC64/dist/stl_wrappers  -DNDEBUG=1 -DTRIMMED=1 -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -IF:/develop/mozilla/central/layout/generic -IF:/develop/mozilla/central/obj/obj-VC64/layout/generic -IF:/develop/mozilla/central/obj/obj-VC64/ipc/ipdl/_ipdlheaders -IF:/develop/mozilla/central/ipc/chromium/src -IF:/develop/mozilla/central/ipc/glue -IF:/develop/mozilla/central/dom/plugins/base -IF:/develop/mozilla/central/layout/base -IF:/develop/mozilla/central/layout/forms -IF:/develop/mozilla/central/layout/style -IF:/develop/mozilla/central/layout/svg -IF:/develop/mozilla/central/layout/tables -IF:/develop/mozilla/central/layout/xul -IF:/develop/mozilla/central/dom/base -IF:/develop/mozilla/central/dom/html -IF:/develop/mozilla/central/dom/xul -IF:/develop/mozilla/central/obj/obj-VC64/dist/include  -IF:/develop/mozilla/central/obj/obj-VC64/dist/include/nspr -IF:/develop/mozilla/central/obj/obj-VC64/dist/include/nss        -MD           -FI F:/develop/mozilla/central/obj/obj-VC64/mozilla-config.h -DMOZILLA_CLIENT   -TP -nologo -Zc:sizedDealloc- -Zc:threadSafeInit- -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -FS -wd4251 -wd4244 -wd4267 -wd4345 -wd4351 -wd4800 -wd4819 -we4553 -GR-  -Zi -UDEBUG -DNDEBUG -O2 -Qpar -Gw -Oy  -IF:/develop/mozilla/central/obj/obj-VC64/dist/include/cairo -GL -Fdgenerated.pdb   F:/develop/mozilla/central/layout/generic/nsLineLayout.cpp

F:/develop/mozilla/central/obj/obj-VC64/_virtualenv/Scripts/python.exe F:/develop/mozilla/central/config/expandlibs_exec.py --uselist -- link -NOLOGO -DLL -OUT:xul.dll -PDB:xul.pdb -SUBSYSTEM:WINDOWS,6.01 -MACHINE:X64  StaticXULComponentsStart.obj nsDllMain.obj  ./module.res -LARGEADDRESSAWARE -NXCOMPAT -DYNAMICBASE  -DEBUG -DEBUGTYPE:CV -DEBUG -OPT:REF,ICF -LTCG:incremental -DELAYLOAD:comdlg32.dll -DELAYLOAD:dbghelp.dll -DELAYLOAD:PowrProf.dll -DELAYLOAD:psapi.dll -DELAYLOAD:rasapi32.dll -DELAYLOAD:rasdlg.dll -DELAYLOAD:secur32.dll -DELAYLOAD:wininet.dll -DELAYLOAD:winspool.drv -DELAYLOAD:api-ms-win-core-winrt-l1-1-0.dll -DELAYLOAD:api-ms-win-core-winrt-string-l1-1-0.dll    ../../memory/volatile/memory_volatile.lib ../../media/libstagefright/media_libstagefright.lib ../../security/certverifier/security_certverifier.lib ../../security/pkix/mozillapkix.lib ../../security/apps/security_apps.lib ../../xpcom/typelib/xpt/xpt.lib ../../xpcom/string/xpcom_string.lib ../../xpcom/base/xpcom_base.lib ../../xpcom/ds/xpcom_ds.lib ../../xpcom/io/xpcom_io.lib ../../xpcom/components/xpcom_components.lib ../../xpcom/threads/xpcom_threads.lib ../../xpcom/reflect/xptinfo/xpcom_reflect_xptinfo.lib ../../xpcom/reflect/xptcall/xpcom_reflect_xptcall.lib ../../xpcom/reflect/xptcall/md/win32/xpcom_reflect_xptcall_md_win32.lib ../../chrome/chrome.lib ../../xpcom/build/xpcom_build.lib ../../modules/libpref/modules_libpref.lib ../../intl/hyphenation/hyphen/intl_hyphenation_hyphen.lib ../../intl/hyphenation/glue/intl_hyphenation_glue.lib ../../intl/locale/intl_locale.lib ../../intl/locale/windows/intl_locale_windows.lib ../../intl/lwbrk/intl_lwbrk.lib ../../intl/strres/intl_strres.lib ../../intl/unicharutil/intl_unicharutil.lib ../../intl/unicharutil/util/internal/intl_unicharutil_util_internal.lib ../../intl/uconv/intl_uconv.lib ../../intl/build/intl_build.lib ../../netwerk/base/netwerk_base.lib ../../netwerk/cookie/netwerk_cookie.lib ../../netwerk/dns/netwerk_dns.lib ../../netwerk/dns/mdns/libmdns/netwerk_dns_mdns_libmdns.lib ../../netwerk/socket/netwerk_socket.lib ../../netwerk/mime/netwerk_mime.lib ../../netwerk/streamconv/netwerk_streamconv.lib ../../netwerk/streamconv/converters/netwerk_streamconv_converters.lib ../../netwerk/cache2/netwerk_cache2.lib ../../netwerk/protocol/about/netwerk_protocol_about.lib ../../netwerk/protocol/app/netwerk_protocol_app.lib ../../netwerk/protocol/data/netwerk_protocol_data.lib ../../netwerk/protocol/device/netwerk_protocol_device.lib ../../netwerk/protocol/file/netwerk_protocol_file.lib ../../netwerk/protocol/ftp/netwerk_protocol_ftp.lib ../../netwerk/protocol/http/netwerk_protocol_http.lib ../../netwerk/protocol/res/netwerk_protocol_res.lib ../../netwerk/protocol/viewsource/netwerk_protocol_viewsource.lib ../../netwerk/protocol/websocket/netwerk_protocol_websocket.lib ../../netwerk/protocol/wyciwyg/netwerk_protocol_wyciwyg.lib ../../netwerk/system/win32/netwerk_system_win32.lib ../../netwerk/ipc/netwerk_ipc.lib ../../netwerk/build/netwerk_build.lib ../../extensions/auth/extensions_auth.lib ../../ipc/chromium/ipc_chromium.lib ../../ipc/glue/ipc_glue.lib ../../ipc/ipdl/ipc_ipdl.lib ../../js/ipc/js_ipc.lib ../../hal/hal.lib ../../js/xpconnect/wrappers/js_xpconnect_wrappers.lib ../../js/xpconnect/loader/js_xpconnect_loader.lib ../../js/xpconnect/src/js_xpconnect_src.lib ../../intl/chardet/intl_chardet.lib ../../media/libyuv/libyuv_libyuv/yuv.lib ../../modules/libjar/modules_libjar.lib ../../modules/libjar/zipwriter/modules_libjar_zipwriter.lib ../../storage/storage.lib ../../storage/build/storage_build.lib ../../extensions/cookie/extensions_cookie.lib ../../extensions/permissions/extensions_permissions.lib ../../rdf/base/rdf_base.lib ../../rdf/util/internal/rdf_util_internal.lib ../../rdf/datasource/rdf_datasource.lib ../../rdf/build/rdf_build.lib ../../uriloader/base/uriloader_base.lib ../../uriloader/exthandler/uriloader_exthandler.lib ../../uriloader/prefetch/uriloader_prefetch.lib ../../caps/caps.lib ../../parser/xml/parser_xml.lib ../../parser/htmlparser/parser_htmlparser.lib ../../parser/html/parser_html.lib ../../gfx/2d/gfx_2d.lib ../../gfx/ycbcr/gfx_ycbcr.lib ../../gfx/src/gfx_src.lib ../../gfx/qcms/gfx_qcms.lib ../../gfx/gl/gfx_gl.lib ../../gfx/layers/gfx_layers.lib ../../gfx/thebes/gfx_thebes.lib ../../gfx/ipc/gfx_ipc.lib ../../image/image.lib ../../image/build/image_build.lib ../../image/decoders/image_decoders.lib ../../image/decoders/icon/win/image_decoders_icon_win.lib ../../image/decoders/icon/image_decoders_icon.lib ../../image/encoders/ico/image_encoders_ico.lib ../../image/encoders/png/image_encoders_png.lib ../../image/encoders/jpeg/image_encoders_jpeg.lib ../../image/encoders/bmp/image_encoders_bmp.lib ../../dom/animation/dom_animation.lib ../../dom/base/dom_base.lib ../../dom/archivereader/dom_archivereader.lib ../../dom/bindings/dom_bindings.lib ../../dom/canvas/dom_canvas.lib ../../dom/crypto/dom_crypto.lib ../../dom/encoding/dom_encoding.lib ../../dom/events/dom_events.lib ../../dom/fetch/dom_fetch.lib ../../dom/filesystem/dom_filesystem.lib ../../dom/html/dom_html.lib ../../dom/json/dom_json.lib ../../dom/jsurl/dom_jsurl.lib ../../dom/asmjscache/dom_asmjscache.lib ../../dom/media/dom_media.lib ../../dom/media/encoder/dom_media_encoder.lib ../../dom/media/mediasink/dom_media_mediasink.lib ../../dom/media/mediasource/dom_media_mediasource.lib ../../dom/media/platforms/dom_media_platforms.lib ../../dom/media/platforms/wmf/dom_media_platforms_wmf.lib ../../dom/media/platforms/ffmpeg/ffvpx/dom_media_platforms_ffmpeg_ffvpx.lib ../../dom/media/wave/dom_media_wave.lib ../../dom/media/systemservices/dom_media_systemservices.lib ../../dom/media/raw/dom_media_raw.lib ../../dom/media/fmp4/dom_media_fmp4.lib ../../dom/notification/dom_notification.lib ../../dom/quota/dom_quota.lib ../../dom/security/dom_security.lib ../../dom/storage/dom_storage.lib ../../dom/svg/dom_svg.lib ../../dom/time/dom_time.lib ../../dom/permission/dom_permission.lib ../../dom/plugins/base/dom_plugins_base.lib ../../dom/plugins/ipc/dom_plugins_ipc.lib ../../dom/system/dom_system.lib ../../dom/ipc/dom_ipc.lib ../../dom/workers/dom_workers.lib ../../dom/audiochannel/dom_audiochannel.lib ../../dom/messagechannel/dom_messagechannel.lib ../../dom/power/dom_power.lib ../../dom/promise/dom_promise.lib ../../dom/smil/dom_smil.lib ../../dom/xbl/dom_xbl.lib ../../dom/xml/dom_xml.lib ../../dom/xslt/base/dom_xslt_base.lib ../../dom/xslt/xml/dom_xslt_xml.lib ../../dom/xslt/xpath/dom_xslt_xpath.lib ../../dom/xslt/xslt/dom_xslt_xslt.lib ../../dom/xul/dom_xul.lib ../../dom/xul/templates/dom_xul_templates.lib ../../dom/secureelement/dom_secureelement.lib ../../view/view.lib ../../widget/widget.lib ../../widget/windows/widget_windows.lib ../../editor/txtsvc/editor_txtsvc.lib ../../editor/libeditor/editor_libeditor.lib ../../editor/txmgr/editor_txmgr.lib ../../editor/composer/editor_composer.lib ../../layout/style/layout_style.lib ../../layout/base/layout_base.lib ../../layout/generic/layout_generic.lib ../../layout/forms/layout_forms.lib ../../layout/tables/layout_tables.lib ../../layout/svg/layout_svg.lib ../../layout/xul/layout_xul.lib ../../layout/xul/tree/layout_xul_tree.lib ../../layout/xul/grid/layout_xul_grid.lib ../../layout/ipc/layout_ipc.lib ../../layout/inspector/layout_inspector.lib ../../layout/printing/layout_printing.lib ../../layout/build/layout_build.lib ../../docshell/base/docshell_base.lib ../../docshell/shistory/docshell_shistory.lib ../../docshell/build/docshell_build.lib ../../embedding/components/windowwatcher/embedding_components_windowwatcher.lib ../../embedding/components/appstartup/embedding_components_appstartup.lib ../../embedding/components/find/embedding_components_find.lib ../../embedding/components/webbrowserpersist/embedding_components_webbrowserpersist.lib ../../embedding/components/commandhandler/embedding_components_commandhandler.lib ../../embedding/components/printingui/ipc/embedding_components_printingui_ipc.lib ../../embedding/components/printingui/win/embedding_components_printingui_win.lib ../../embedding/components/build/embedding_components_build.lib ../../embedding/browser/embedding_browser.lib ../../embedding/browser/build/embedding_browser_build.lib ../../xpfe/appshell/xpfe_appshell.lib ../../extensions/universalchardet/src/base/extensions_universalchardet_src_base.lib ../../extensions/universalchardet/src/xpcom/extensions_universalchardet_src_xpcom.lib ../../xpfe/components/windowds/xpfe_components_windowds.lib ../../xpfe/components/directory/xpfe_components_directory.lib ../../xpfe/components/build/xpfe_components_build.lib ../../extensions/spellcheck/hunspell/glue/extensions_spellcheck_hunspell_glue.lib ../../extensions/spellcheck/hunspell/src/extensions_spellcheck_hunspell_src.lib ../../extensions/spellcheck/src/extensions_spellcheck_src.lib ../../security/manager/ssl/security_manager_ssl.lib ../../security/manager/pki/security_manager_pki.lib ../../toolkit/components/alerts/toolkit_components_alerts.lib ../../toolkit/components/commandlines/toolkit_components_commandlines.lib ../../toolkit/components/downloads/toolkit_components_downloads.lib ../../toolkit/components/filewatcher/toolkit_components_filewatcher.lib ../../toolkit/components/finalizationwitness/toolkit_components_finalizationwitness.lib ../../toolkit/components/find/toolkit_components_find.lib ../../toolkit/components/jsdownloads/src/toolkit_components_jsdownloads_src.lib ../../toolkit/components/lz4/toolkit_components_lz4.lib ../../toolkit/components/mediasniffer/toolkit_components_mediasniffer.lib ../../toolkit/components/osfile/toolkit_components_osfile.lib ../../toolkit/components/perf/toolkit_components_perf.lib ../../toolkit/components/places/toolkit_components_places.lib ../../toolkit/components/protobuf/toolkit_components_protobuf.lib ../../toolkit/components/reflect/toolkit_components_reflect.lib ../../toolkit/components/startup/toolkit_components_startup.lib ../../toolkit/components/statusfilter/toolkit_components_statusfilter.lib ../../toolkit/components/typeaheadfind/toolkit_components_typeaheadfind.lib ../../toolkit/components/ctypes/toolkit_components_ctypes.lib ../../toolkit/components/autocomplete/toolkit_components_autocomplete.lib ../../toolkit/components/satchel/toolkit_components_satchel.lib ../../toolkit/components/terminator/toolkit_components_terminator.lib ../../toolkit/components/build/toolkit_components_build.lib ../../toolkit/mozapps/extensions/toolkit_mozapps_extensions.lib ../../toolkit/profile/toolkit_profile.lib ../../toolkit/xre/toolkit_xre.lib ../../toolkit/system/windowsproxy/toolkit_system_windowsproxy.lib ../../extensions/pref/autoconfig/src/extensions_pref_autoconfig_src.lib ../../startupcache/startupcache.lib ../../other-licenses/snappy/other-licenses_snappy.lib ../../memory/fallible/fallible.lib ../../js/src/js_static.lib ../../layout/media/gkmedias.lib ../../config/external/nspr/nspr.lib ../../config/external/sqlite/sqlite.lib ../../config/external/zlib/zlib.lib ../../toolkit/library/StaticXULComponentsEnd/StaticXULComponentsEnd.lib ../../mozglue/build/mozglue.lib ../../security/nss/lib/nss/nss3.lib ../../security/sandbox/win/src/sandboxbroker/sandboxbroker.lib ../../config/external/lgpllibs/lgpllibs.lib    kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib secur32.lib netapi32.lib psapi.lib shell32.lib dbghelp.lib rpcrt4.lib usp10.lib ole32.lib msimg32.lib winmm.lib mfuuid.lib wmcodecdspuuid.lib strmiids.lib version.lib winspool.lib  comdlg32.lib imm32.lib shlwapi.lib ws2_32.lib rasapi32.lib rasdlg.lib iphlpapi.lib uxtheme.lib setupapi.lib secur32.lib sensorsapi.lib portabledeviceguids.lib windowscodecs.lib wininet.lib wbemuuid.lib wintrust.lib wtsapi32.lib sapi.lib oleaut32.lib delayimp.lib
Could you provide the config file you use to reproduce this issue?

FWIW, I did a half PGO build (only the first phase, no profiling and second phase), which, IIUC, should have WPO enabled as well. But that doesn't seem to crash at startup for me.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #11)
> Could you provide the config file you use to reproduce this issue?
> 
> FWIW, I did a half PGO build (only the first phase, no profiling and second
> phase), which, IIUC, should have WPO enabled as well. But that doesn't seem
> to crash at startup for me.

Actually I made a lot of changes to the source. Before bug 1230034 it works well. After that, non-wpo 64 build works well and wpo 64 builds crash at startup. 
It's an issue of optimization. First phase builds don't get optimization, you need get the second phase one.
But if you can't reproduce this, I will mark this as invalid as it may relate to my local changes.
Like bug 555727, it works after removing MOZ_CONSTEXPR
Set MOZ_PGO=1 in your environment and do a clobber `mach build` to perform a PGO build. I reliably get a crash in xpcshell.exe as part of the build when processing the startup cache with VS2015. Not sure if that is this bug or another one.
(In reply to Gregory Szorc [:gps] from comment #14)
> Set MOZ_PGO=1 in your environment and do a clobber `mach build` to perform a
> PGO build. I reliably get a crash in xpcshell.exe as part of the build when
> processing the startup cache with VS2015. Not sure if that is this bug or
> another one.

Could you try the method mentioned in comment 13 to see whether the crash still happens?

I think that was saying, remove the MOZ_CONSTEXPR in these three lines: https://dxr.mozilla.org/mozilla-central/rev/d848a5628d801a460a7244cbcdea22d328d8b310/layout/generic/nsIFrame.h#853,861,868
Flags: needinfo?(gps)
Removing MOZ_CONSTEXPR as instructed in comment #15 did not make the xpcshell.exe crash go away.
Flags: needinfo?(gps)
Then probably not the same issue. Also I guess xpcshell doesn't actually link to xul.dll, so nsIFrame could do nothing about xpcshell. Probably you want to submit a new bug for the issue you met with pgo.
(In reply to Gregory Szorc [:gps] from comment #16)
> Removing MOZ_CONSTEXPR as instructed in comment #15 did not make the
> xpcshell.exe crash go away.

It turns out the xpcshell.exe crash was due to a DLL path issue. I have a patch for that in bug 1251313. With that applied, I /think/ I can repro the crash in this bug. I'm getting a null pointer dereference on the 7th call to the following code in nsFrame.cpp:

nsFrame::MarkIntrinsicISizesDirty()
{
  // This version is meant only for what used to be box-to-block adaptors.
  // It should not be called by other derived classes.
  if (::IsBoxWrapped(this)) {
    nsBoxLayoutMetrics *metrics = BoxMetrics();

    SizeNeedsRecalc(metrics->mPrefSize);
                    ^ nullptr

We're hitting the following block in void* FramePropertyTable::GetInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty, bool* aFoundResult)

  if (!entry)
    return nullptr;

This reproduces 100% of the time.

My C++ skills are a bit rusty. I'll trust to muck around with the MOZ_CONSTEXPR foo again and see if that makes the crash go away.
The suggestion in comment #15 fixes the startup crash!
Then that's do it. If changing that to "const" doesn't reintroduce this issue, we probably should do so instead of only removing "MOZ_CONSTEXPR".
s/that's/let's
PGO builds on Visual Studio 2015 Update 1 did not take kindly to
MOZ_CONSTEXPR in this file, causing a startup crash (for reasons
I can't explain). Switching to literal "const" makes the crash
go away.

Review commit: https://reviewboard.mozilla.org/r/37191/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37191/
Attachment #8724847 - Flags: review?(dbaron)
https://reviewboard.mozilla.org/r/37191/#review33843

::: layout/generic/nsIFrame.h:851
(Diff revision 1)
> +// Use of MOZ_CONSTEXPR caused startup crashes with MSVC2015 PGO builds.

Probably worth mentioning it's MSVC2015u1. Their release notes of Update 2 CTP mention "constexpr improvements", thus hopefully this issue has been fixed there. We may want to revert this change when we drop u1 support. I don't think using const could hurt anything, though.
Comment on attachment 8724847 [details]
MozReview Request: Bug 1244006 - Use const instead of MOZ_CONSTEXPR to avoid startup crash; r?dbaron

https://reviewboard.mozilla.org/r/37191/#review33867

::: layout/generic/nsIFrame.h:851
(Diff revision 1)
> +// Use of MOZ_CONSTEXPR caused startup crashes with MSVC2015 PGO builds.

Could you make this a /* C style comment */ and repeat it 3 times, inside the macro, directly above the line with const?  (And also address Xidorn's comment.)  You'll probably need to wrap the line, which means, I think, that each line needs to be its own /* C style comment */.

(Also, I'd have been fine with Xidorn reviewing this.)
Attachment #8724847 - Flags: review?(dbaron) → review+
Comment on attachment 8724847 [details]
MozReview Request: Bug 1244006 - Use const instead of MOZ_CONSTEXPR to avoid startup crash; r?dbaron

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37191/diff/1-2/
Not sure why you didn't land the patch. Probably I didn't make it clear in IRC that this patch looks good to me.

Anyway, I've landed that patch for you via autoland, which looks promising now :)
https://hg.mozilla.org/mozilla-central/rev/a0f0115cbc71
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Assignee: nobody → gps
You need to log in before you can comment on or make changes to this bug.