Last Comment Bug 657569 - MSVC miscompiles nptest.dll with optimization but no PGO
: MSVC miscompiles nptest.dll with optimization but no PGO
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All Windows 7
: -- normal (vote)
: mozilla8
Assigned To: Mike Hommey [:glandium]
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on:
Blocks: 658313
  Show dependency treegraph
 
Reported: 2011-05-17 03:02 PDT by Mike Hommey [:glandium]
Modified: 2011-07-10 23:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Build nptest.dll without optimization on windows (918 bytes, patch)
2011-07-05 22:59 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Build nptest.dll without optimization on windows (909 bytes, patch)
2011-07-06 06:55 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2011-05-17 03:02:44 PDT
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 Jesse Ruderman 2011-05-17 22:39:07 PDT
How did you determine that this is a compiler bug rather than an undefined-behavior bug in the source code?
Comment 2 Mike Hommey [:glandium] 2011-05-17 22:51:16 PDT
(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.
Comment 3 Mike Hommey [:glandium] 2011-07-05 22:59:52 PDT
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.
Comment 4 Mike Hommey [:glandium] 2011-07-06 04:04:04 PDT
Comment on attachment 544158 [details] [diff] [review]
Build nptest.dll without optimization on windows

This doesn't work, for obvious reasons, actually.
Comment 5 Mike Hommey [:glandium] 2011-07-06 06:55:14 PDT
Created attachment 544229 [details] [diff] [review]
Build nptest.dll without optimization on windows

This one works
Comment 6 Ted Mielczarek [:ted.mielczarek] 2011-07-08 10:51:48 PDT
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"
Comment 7 Mike Hommey [:glandium] 2011-07-10 23:15:59 PDT
http://hg.mozilla.org/mozilla-central/rev/20076ad56750

Note You need to log in before you can comment on or make changes to this bug.