Closed Bug 975803 Opened 6 years ago Closed 6 years ago

Internal Compiler Error on building jsscript.cpp with MSVC10

Categories

(Core :: JavaScript Engine, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: emk, Assigned: emk)

References

Details

Attachments

(1 file, 2 obsolete files)

0:39.75 h:\m\mozilla-central\js\src\jsscript.cpp(947) : fatal error C1001: コン
パイラで内部エラーが発生しました。
 0:39.75 (コンパイラ ファイル 'f:\dd\vctools\compiler\utc\src\p2\main.c[0x60BFBB
A0:0x00000004]'、行 183)
 0:39.75  この問題を回避するには、上記の場所付近のプログラムを単純化するか変更し
てください。
 0:39.75 詳細については、Visual C++ ヘルプ メニューのサポート情報コマンドを
 0:39.75 選択してください。またはサポート情報 ヘルプ ファイルを参照してください
。
 0:39.75 c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\BIN\cl.exe での
内部コンパイラ エラーです。エラー報告をマイクロソフトに送信するために、後でメッ
セージが表示されます。
Build succeeded when I commented out the following three lines:
>          default: {
>            MOZ_ASSUME_UNREACHABLE("Unknown class kind.");
>            return false;
>          }
Regression from bug 920322?
Blocks: 920322
Attached patch Workaround a MSVC10 compiler bug (obsolete) — Splinter Review
This patch should be safe because this case will never be reached by definition :)
Attachment #8380333 - Flags: review?(luke)
Comment on attachment 8380333 [details] [diff] [review]
Workaround a MSVC10 compiler bug

Sorry, the logic was reversed. I'll post a fix later.
Attachment #8380333 - Flags: review?(luke)
I don't think this is the problem that you are seeing here, because this pattern appear in many places in the code base.  Also, this code is here for safety reason and for fuzzers, so I do not think this is a good idea to disable it on MSVC.

Can you reproduce it with newer version of MSVC?
Could moving this default statement higher in the switch work as a work-around?  Is this issue related to the "return false"?
(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> I don't think this is the problem that you are seeing here, because this
> pattern appear in many places in the code base.

This is an unexpected compiler bug, so any sane pattern will not be expected.
Surely msvc10 silenced with the default: block commented out and complained wit the block.

> Also, this code is here for safety reason

If the code reaches MOZ_ASSUME_UNREACHABLE, it is unsafe anyway. See the big comment at the beginning of MOZ_ASSUME_UNREACHABLE. If you need safety, Please use MOZ_CRASH instead. Or MOZ_RUNTIME_ASSERT or MOZ_ASSERT or something.

> and for fuzzers, so I do not think this is a good idea to
> disable it on MSVC.

Of course I had not done this if the compiler had not a bug. It's the reason why I'm trying to narrow the compiler version as much as possible.

> Can you reproduce it with newer version of MSVC?

No. It works with msvc12. But conversely, MS will not fix this bug with msvc10 anymore. So I have to write a workaround.

> Could moving this default statement higher in the switch work as a
> work-around?

No.

> Is this issue related to the "return false"?

Oh, only commenting out "return false" worked. I'll update the patch to narrow down the #ifdef'ed range.
Attached patch Workaround a MSVC10 compiler bug (obsolete) — Splinter Review
I actually confirmed it compiles this time.
Attachment #8380333 - Attachment is obsolete: true
Attachment #8380582 - Flags: review?(luke)
I'd rather not add the #if cruft for such a corner case since it's pretty ugly.  Ordinarily, I'd just recommend removing the 'return false', but this is serialization so I'd rather be more conservative and fail safely if we get weird bits.  So instead, can we replace the MOZ_ASSUME_UNREACHABLE with MOZ_ASSERT(false, "Unreachable")?  (I assume that fixes the ICE.)
Yes, MOZ_ASSERT also works.
Attachment #8380582 - Attachment is obsolete: true
Attachment #8380582 - Flags: review?(luke)
Attachment #8380969 - Flags: review?(luke)
Attachment #8380969 - Flags: review?(luke) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d20fd7b8501a
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/d20fd7b8501a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.