1395907 - Require MSVC 15.3 to build on Windows (run-time crash: "aCounterStyle-> was nullptr" crash at CounterStylePtr& operator=(CounterStyle* aCounterStyle))

Reporter

Description

•

7 years ago

Win, debug x64 build, clean profile, 100% repro few secs after startup:

>	xul.dll!mozilla::CounterStylePtr::operator=(0x00007ff95f228390) Line 221	C++
 	xul.dll!nsStyleList::nsStyleList(0x0000014d6ad77800) Line 624	C++
 	xul.dll!nsRuleNode::SetDefaultOnRoot(eStyleStruct_List, 0x0000014d6ad9e478) Line 2909	C++
 	xul.dll!nsRuleNode::WalkRuleTree(eStyleStruct_List, 0x0000014d6ad9e478) Line 2818	C++
 	xul.dll!nsRuleNode::GetStyleData(eStyleStruct_List, 0x0000014d6ad9e478, true) Line 10480	C++
 	xul.dll!mozilla::GeckoStyleContext::StyleData(eStyleStruct_List) Line 1089	C++
 	xul.dll!nsRuleNode::WalkRuleTree(eStyleStruct_List, 0x0000014d6c6e8f00) Line 2807	C++
 	xul.dll!nsRuleNode::GetStyleData(eStyleStruct_List, 0x0000014d6c6e8f00, true) Line 10480	C++
 	xul.dll!mozilla::GeckoStyleContext::StyleData(eStyleStruct_List) Line 1089	C++
 	xul.dll!nsRuleNode::WalkRuleTree(eStyleStruct_List, 0x0000014d6caf2810) Line 2807	C++
 	xul.dll!nsRuleNode::GetStyleData(eStyleStruct_List, 0x0000014d6caf2810, true) Line 10480	C++
 	xul.dll!mozilla::GeckoStyleContext::StyleData(eStyleStruct_List) Line 1089	C++
 	xul.dll!nsRuleNode::WalkRuleTree(eStyleStruct_List, 0x0000014d6cbbf868) Line 2807	C++
 	xul.dll!nsRuleNode::GetStyleData(eStyleStruct_List, 0x0000014d6cbbf868, true) Line 10480	C++
 	xul.dll!mozilla::GeckoStyleContext::StyleData(eStyleStruct_List) Line 1089	C++
 	xul.dll!nsRuleNode::WalkRuleTree(eStyleStruct_List, 0x0000014d6cbc00b8) Line 2807	C++
 	xul.dll!nsRuleNode::GetStyleList<1>(0x0000014d6cbc00b8, 3917010173952) Line 51	C++
 	xul.dll!nsStyleContext::DoGetStyleList<1>() Line 51	C++
 	xul.dll!nsStyleContext::StyleList() Line 51	C++
 	xul.dll!nsIFrame::StyleList() Line 51	C++
 	xul.dll!nsImageBoxFrame::DidSetStyleContext(0x0000000000000000) Line 535	C++
 	xul.dll!nsFrame::Init(0x0000014d6bbf2d30, 0x0000014d6cbbff40, 0x0000000000000000) Line 702	C++
 	xul.dll!nsLeafBoxFrame::Init(0x0000014d6bbf2d30, 0x0000014d6cbbff40, 0x0000000000000000) Line 62	C++
 	xul.dll!nsImageBoxFrame::Init(0x0000014d6bbf2d30, 0x0000014d6cbbff40, 0x0000000000000000) Line 209	C++
 	xul.dll!nsCSSFrameConstructor::InitAndRestoreFrame({...}, 0x0000014d6bbf2d30, 0x0000014d6cbbff40, 0x0000014d6cbc05d8, true) Line 5133	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal({...}, {...}, 0x0000014d6cbbff40, {...}) Line 4048	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem({...}, {...}, 0x0000014d6cbbff40, {...}) Line 6412	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList({...}, {...}, 0x0000014d6cbbff40, false, {...}) Line 11074	C++
 	xul.dll!nsCSSFrameConstructor::ProcessChildren({...}, 0x0000014d6bbf2ca0, 0x0000014d6cbbf868, 0x0000014d6cbbff40, true, {...}, false, 0x0000000000000000, 0x0000014d6cbbff40) Line 11391	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal({...}, {...}, 0x0000014d6cbbf450, {...}) Line 4209	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem({...}, {...}, 0x0000014d6cbbf450, {...}) Line 6412	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList({...}, {...}, 0x0000014d6cbbf450, false, {...}) Line 11074	C++
 	xul.dll!nsCSSFrameConstructor::ProcessChildren({...}, 0x0000014d6bbf2af0, 0x0000014d6caf2810, 0x0000014d6cbbf450, true, {...}, false, 0x0000000000000000, 0x0000014d6cbbf450) Line 11391	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal({...}, {...}, 0x0000014d6c7afac0, {...}) Line 4209	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem({...}, {...}, 0x0000014d6c7afac0, {...}) Line 6412	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList({...}, {...}, 0x0000014d6c7afac0, false, {...}) Line 11074	C++
 	xul.dll!nsCSSFrameConstructor::ProcessChildren({...}, 0x0000014d6b3b2ca0, 0x0000014d6c6e8f00, 0x0000014d6c7afac0, true, {...}, false, 0x0000000000000000, 0x0000014d6c7afac0) Line 11391	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal({...}, {...}, 0x0000014d6ad9ea90, {...}) Line 4209	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem({...}, {...}, 0x0000014d6ad9ea90, {...}) Line 6412	C++
 	xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList({...}, {...}, 0x0000014d6ad9ea90, false, {...}) Line 11074	C++
 	xul.dll!nsCSSFrameConstructor::ProcessChildren({...}, 0x0000014d6b7d47d0, 0x0000014d6ad9e478, 0x0000014d6ad9ea90, true, {...}, false, 0x0000000000000000, 0x0000014d6ad9ea90) Line 11391	C++
 	xul.dll!nsCSSFrameConstructor::ConstructDocElementFrame(0x0000014d6b7d47d0, 0x0000000000000000) Line 2798	C++
 	xul.dll!nsCSSFrameConstructor::ContentRangeInserted(0x0000000000000000, 0x0000014d6b7d47d0, 0x0000000000000000, 0x0000000000000000, false, false, 0x0000000000000000) Line 8122	C++
 	xul.dll!nsCSSFrameConstructor::ContentRangeInserted(0x0000000000000000, 0x0000014d6b7d47d0, 0x0000000000000000, 0x0000000000000000, false, 0x0000000000000000) Line 284	C++
 	xul.dll!nsCSSFrameConstructor::ContentInserted(0x0000000000000000, 0x0000014d6b7d47d0, 0x0000000000000000, false) Line 8014	C++
 	xul.dll!mozilla::PresShell::Initialize(7200, 0) Line 1818	C++
 	xul.dll!mozilla::dom::XULDocument::StartLayout() Line 1941	C++
 	xul.dll!mozilla::dom::XULDocument::DoneWalking() Line 3033	C++
 	xul.dll!mozilla::dom::XULDocument::ResumeWalk() Line 2977	C++
 	xul.dll!mozilla::dom::XULDocument::OnPrototypeLoadDone(true) Line 595	C++
 	xul.dll!mozilla::dom::XULDocument::EndLoad() Line 571	C++
 	xul.dll!XULContentSinkImpl::DidBuildModel(false) Line 228	C++
 	xul.dll!nsParser::DidBuildModel(NS_OK) Line 491	C++
 	xul.dll!nsParser::ResumeParse(true, true, true) Line 1102	C++
 	xul.dll!nsParser::OnStopRequest(0x0000014d6a8f86f0, 0x0000000000000000, NS_OK) Line 1475	C++
 	xul.dll!nsBaseChannel::OnStopRequest(0x0000014d6c68ef00, 0x0000000000000000, NS_OK) Line 857	C++
 	xul.dll!nsInputStreamPump::OnStateStop() Line 731	C++
 	xul.dll!nsInputStreamPump::OnInputStreamReady(0x0000014d6c71aa60) Line 447	C++
 	xul.dll!nsInputStreamReadyEvent::Run() Line 99	C++
 	xul.dll!nsThread::ProcessNextEvent(false, 0x000000112a3fea32) Line 1040	C++
 	xul.dll!NS_ProcessNextEvent(0x0000014d64114350, false) Line 521	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(0x0000014d6418d070) Line 97	C++
 	xul.dll!MessageLoop::RunInternal() Line 327	C++
 	xul.dll!MessageLoop::RunHandler() Line 320	C++
 	xul.dll!MessageLoop::Run() Line 300	C++
 	xul.dll!nsBaseAppShell::Run() Line 160	C++
 	xul.dll!nsAppShell::Run() Line 230	C++
 	xul.dll!nsAppStartup::Run() Line 288	C++
 	xul.dll!XREMain::XRE_mainRun() Line 4646	C++
 	xul.dll!XREMain::XRE_main(4, 0x0000014d641020a0, {...}) Line 4810	C++
 	xul.dll!XRE_main(4, 0x0000014d641020a0, {...}) Line 4905	C++
 	xul.dll!mozilla::BootstrapImpl::XRE_main(4, 0x0000014d641020a0, {...}) Line 46	C++
 	firefox.exe!do_main(4, 0x0000014d641020a0, 0x0000014d63dc66a0) Line 237	C++
 	firefox.exe!NS_internal_main(4, 0x0000014d641020a0, 0x0000014d63dc66a0) Line 309	C++
 	firefox.exe!wmain(4, 0x0000014d63db9c30) Line 115	C++
 	firefox.exe!__scrt_common_main_seh() Line 259	C++
 	kernel32.dll!BaseThreadInitThunk()	Unknown
 	ntdll.dll!RtlUserThreadStart()	Unknown

Honza Bambas (:mayhemer)

Reporter

Comment 1

•

7 years ago

Pressed enter too soon: 

m-c@a3585c77e2b1 and it crashes at the MOZ_ASSERT(!aCounterStyle->AsAnonymous()); eval - aCounterStyle is broken:

-		aCounterStyle	xul.dll!0x00007ff95f228390 {mStyle=2 }	mozilla::CounterStyle *
+		__vfptr	0x0000000000000000 {???, ???, ???, ???, ???, ???, ???, ???, ???, ???, ???, ???, ???, ???, ???}	void * *
		mStyle	2	const int

Severity: normal → blocker

Daosheng Mu[:daoshengmu]

Comment 2

•

7 years ago

It happens to me as well.

Alex Gaynor [:Alex_Gaynor]

Comment 3

•

7 years ago

More discussion of this issue at: https://bugzilla.mozilla.org/show_bug.cgi?id=1393189#c53

Alex Gaynor [:Alex_Gaynor]

Comment 4

•

7 years ago

Confirmed that I was able to resolve this by upgrading MSVC.

Daosheng Mu[:daoshengmu]

Comment 5

•

7 years ago

After updating MSVC to 15.3, it fixes my problem.

Astley Chen (inactive)

Comment 6

•

7 years ago

Is this bug still valid or worth a fix?

Honza Bambas (:mayhemer)

Reporter

Comment 7

•

7 years ago

I can confirm that a build made with MSVC 15.3+ fixes the crash.  I think we should make that version a minimum requirement to build on Windows, at least.  Why this crash is present would be of course interesting to know, but it's probably out of scope/resources.

Component: Layout → Build Config

Summary: "aCounterStyle-> was nullptr" crash at CounterStylePtr& operator=(CounterStyle* aCounterStyle) → Require MSVC 15.3 to build on Windows (run-time crash: "aCounterStyle-> was nullptr" crash at CounterStylePtr& operator=(CounterStyle* aCounterStyle))

Honza Bambas (:mayhemer)

Reporter

Comment 8

•

7 years ago

(IMO, marking this WONTFIX is good too...)

Gregory Szorc [:gps]

Assignee

Comment 9

•

7 years ago

I'm fine bumping the minimum Visual Studio 2017 version so others don't needlessly run into this. Since we don't have CI coverage of VS2017, this only impacts developers.

I'd write a patch, but I don't have access to a Windows machine at the moment to test this. I /think/ this is a one-liner in build/moz.configure/toolchain.configure (search for "vswhere") to add a "-version" argument.

However, the trivial change may not be one I would make in isolation. I /thought/ that configure printed info about a too-old VS minor version so we could enforce minimum update levels. But I can find no such code in configure. Was it (accidentally) removed as part of porting to moz.configure? Or am I just not seeing it? Anyway, we definitely want the error to say something along the lines of "detected vs too old; got XXX; need YYY" instead of "vs 2017 not found."

Blocks: vs2017

Masatoshi Kimura [:emk]

Comment 10

•

7 years ago

So we can't use the compiler that CI uses?

Gregory Szorc [:gps]

Assignee

Comment 11

•

7 years ago

CI currently uses Visual Studio 2015 Update 3 14.0.25425.01 / SDK 10.0.14393.0.

You /could/ download a zip file if you have access to the internal tooltool server. And you could point your mozconfig at it with enough mozconfig hacks (bug 1401627 makes the mozconfig bits a bit easier).

Masatoshi Kimura [:emk]

Comment 12

•

7 years ago

(In reply to Gregory Szorc [:gps] from comment #11)
> CI currently uses Visual Studio 2015 Update 3 14.0.25425.01 / SDK
> 10.0.14393.0.

Then, if you require VS 2017, developers can no longer use the CI version (that is, VS 2015) regardless of the update version. No?

Please upgrade CI before forcing us to use VS 2017.

> You /could/ download a zip file if you have access to the internal tooltool
> server. And you could point your mozconfig at it with enough mozconfig hacks
> (bug 1401627 makes the mozconfig bits a bit easier).

Unless you impose VS 2017. Moreover, I have no access to the internal tooltool server. (I'm not a Mozilla employee.)

Gregory Szorc [:gps]

Assignee

Comment 13

•

7 years ago

The build system currently accepts either 2015 or 2017. That's not going to change until we switch CI to building with VS2017. At some point thereafter, we likely require VS2017 to keep things simpler.

Xidorn Quan [:xidorn] UTC+10

Comment 14

•

7 years ago

(In reply to Gregory Szorc [:gps] from comment #13)
> The build system currently accepts either 2015 or 2017. That's not going to
> change until we switch CI to building with VS2017. At some point thereafter,
> we likely require VS2017 to keep things simpler.

I think people are asking bumping the minimum version to 2015u3 for local to match what CI currently uses.

Bumping to requiring 2017 is a separate issue.

Mike Hommey [:glandium]

Comment 15

•

7 years ago

The bug explicitly says MSVC 15.3. MSVC versions are confusing... 15.3 is 2017u3, not 2015u3.

Xidorn Quan [:xidorn] UTC+10

Comment 16

•

7 years ago

(In reply to Mike Hommey [:glandium] from comment #15)
> The bug explicitly says MSVC 15.3. MSVC versions are confusing... 15.3 is
> 2017u3, not 2015u3.

Oh, hmmm. At least bumping the requirement to what CI uses would be useful, since that works anyway.

Gregory Szorc [:gps]

Assignee

Comment 17

•

7 years ago

I thought we already required vs2015u3+. But on a 2nd examination of the code, I can't actually find where we do this. So either I'm imagining something or it got removed.

We should absolutely have configure validate a minimum visual studio update level. And for VS2015 it should be >=14.0.25425.

Gregory Szorc [:gps]

Assignee

Comment 18

•

7 years ago

I found a Windows VM on my laptop. Will try to fix this.

Assignee: nobody → gps

Status: NEW → ASSIGNED

Comment hidden (mozreview-request)

Automation has used Visual Studio 2015 Update 3 for ages. I /thought/
we had a minimum version check somewhere, but I couldn't find it.

There is also a known compilation bug that is fixed in Visual Studio
2017 15.3.

So, this commit establishes minimum version requirements for Visual
Studio 2015 and 2017 to ensure modern and more supported toolchains
are used.

Review commit: https://reviewboard.mozilla.org/r/183546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183546/

Gregory Szorc [:gps]

Assignee

Comment 20

•

7 years ago

I haven't actually tested this change. Windows VM is super slow :/

I may give up on the VM and test on an AWS instance. But if someone has a Windows install with Visual Studio 2015 and 2017 floating around and wants to give it a go, test with --with-visual-studio-version=2015, --with-visual-studio-version=2017, and no option. Verify configure works. Then, adjust the minimum versions in the source code modified in this commit and verify that minimum version enforcement works.

Mike Hommey [:glandium]

Comment 21

•

7 years ago

mozreview-review

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

https://reviewboard.mozilla.org/r/183546/#review188696

::: commit-message-e6b34:3
(Diff revision 1)
> +Automation has used Visual Studio 2015 Update 3 for ages. I /thought/
> +we had a minimum version check somewhere, but I couldn't find it.

https://dxr.mozilla.org/mozilla-central/rev/33b7b8e81b4befcba503c0e48cd5370aeb715085/build/moz.configure/toolchain.configure#758

It would be better if we didn't spread those version checks too much... IOW, it would be better to check the msvc 2017 version there.

Gregory Szorc [:gps]

Assignee

Comment 22

•

7 years ago

mozreview-review-reply

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

https://reviewboard.mozilla.org/r/183546/#review188696

> https://dxr.mozilla.org/mozilla-central/rev/33b7b8e81b4befcba503c0e48cd5370aeb715085/build/moz.configure/toolchain.configure#758
> 
> It would be better if we didn't spread those version checks too much... IOW, it would be better to check the msvc 2017 version there.

Oh, there it is! I must be blind. I must have missed it because I was searching for {2015, 2017, 14., 15.} and not 19. The Visual Studio version numbers are something else, aren't they.

Comment hidden (mozreview-request)

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183546/diff/1-2/

Mike Hommey [:glandium]

Comment 24

•

7 years ago

mozreview-review

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

https://reviewboard.mozilla.org/r/183546/#review189088

::: build/moz.configure/toolchain.configure:771
(Diff revision 2)
> +            # Starting with VS2017 15.3, MSC_VER increments with every
> +            # release. So VS2017 15.3 is 19.11. Next will be 19.12, etc.

Is that really going to be true, and for how long? I'd rather leave this comment out.

Attachment #8912163 - Flags: review?(mh+mozilla) → review+

Gregory Szorc [:gps]

Assignee

Comment 25

•

7 years ago

mozreview-review-reply

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

https://reviewboard.mozilla.org/r/183546/#review189088

> Is that really going to be true, and for how long? I'd rather leave this comment out.

https://blogs.msdn.microsoft.com/vcblog/2016/10/05/visual-c-compiler-version/

But I'll remove the comment.

Comment hidden (mozreview-request)

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183546/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8912163 [details]
Bug 1395907 - Require Visual Studio 2017 15.3+;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183546/diff/3-4/

Pulsebot

Comment 28

•

7 years ago

Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bac67e7e708d
Require Visual Studio 2017 15.3+; r=glandium

Phil Ringnalda (:philor)

Comment 29

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/bac67e7e708d

Status: ASSIGNED → RESOLVED

Closed: 7 years ago

status-firefox58: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla58

Sylvestre Ledru [:Sylvestre]

Updated

•

7 years ago

status-firefox57: affected → wontfix

BMO Automation

Updated

•

7 years ago

Product: Core → Firefox Build System