MSVC miscompiles nptest.dll with optimization but no PGO

RESOLVED FIXED in mozilla8

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla8
All
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
I noticed this when I got a perma orange on try:
REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/632423-1.html | This test left crash dumps behind, but we weren't expecting it to!
REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/layout/reftests/bugs/632423-1.html | image comparison (==)

What I had been doing was to disable PGO on my try build to avoid waiting too long for the build.

Then, I reproduced locally, and under the debugger, the failure wouldn't make much sense. So I tried rebuilding nptest.dll only with "MOZ_OPTIMIZE_FLAGS=" ; it started working. So I tried again rebuilding nptest.dll with PGO, and it worked too.

So it only fails when building with optimization but without PGO, which is the setup most developers would be using locally...

I guess we could add MODULE_OPTIMIZE_FLAGS= somewhere in modules/plugin/test/testplugin/Makefile.in...

Comment 1

6 years ago
How did you determine that this is a compiler bug rather than an undefined-behavior bug in the source code?
(Assignee)

Comment 2

6 years ago
(In reply to comment #1)
> How did you determine that this is a compiler bug rather than an
> undefined-behavior bug in the source code?

It crashes on nptest.cpp:775, which reads:
  scriptableObject->drawColor = parseHexColor(argv[i], strlen(argv[i]));

It crashes with a completely broken value of argv.

When entering the loop at nptest.cpp:769, all values of argn, argc, and argv are good. The only args that'd match in the loop for 632423-1.html would be drawmode, and color.

That's either a compiler bug or something screwing up the stack badly. The latter would be very likely to screw things up on mac and linux as well, but obviously don't.

I haven't looked at the assembly, though.
(Assignee)

Comment 3

6 years ago
Created attachment 544158 [details] [diff] [review]
Build nptest.dll without optimization on windows

Let's go this way. If someone feels like we need to spend time understanding what is actually going on for the sake of a test plugin, they can clone this bug.
Attachment #544158 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

6 years ago
Blocks: 658313
(Assignee)

Updated

6 years ago
Assignee: nobody → mh+mozilla
(Assignee)

Comment 4

6 years ago
Comment on attachment 544158 [details] [diff] [review]
Build nptest.dll without optimization on windows

This doesn't work, for obvious reasons, actually.
Attachment #544158 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 5

6 years ago
Created attachment 544229 [details] [diff] [review]
Build nptest.dll without optimization on windows

This one works
Attachment #544229 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

6 years ago
Attachment #544158 - Attachment is obsolete: true
Comment on attachment 544229 [details] [diff] [review]
Build nptest.dll without optimization on windows

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

::: dom/plugins/test/testplugin/Makefile.in
@@ +98,5 @@
>  RESFILE   = nptest.res
>  DEFFILE   = $(win_srcdir)/nptest.def
>  OS_LIBS  += $(call EXPAND_LIBNAME,msimg32)
> +
> +# Windows optim builds without PGO break nptest.dll

"opt" or "optimized"
Attachment #544229 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/mozilla-central/rev/20076ad56750
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.