408258 - [valgrind] scriptableIO.xul test failures valgrind errors when compiled with bug 407794 settings

Status: RESOLVED WONTFIX

(Core :: General, defect)

Description

[Robert Sayre]

This can be reproduced by running

perl runtests.pl --chrome, and visiting the scriptableIO.xul test.

CCing roc, because there are also textrun warnings present in the valgrind log.

Comment 1

[Robert Sayre]

Attachment: valgrind log

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It looks like some uninitialized data gets set as text hereabouts:

==4592== Conditional jump or move depends on uninitialised value(s)
==4592==    at 0x168F61A1: nsTextFragment::SetTo(unsigned short const*, int) (in /home/sayrer/dev/opt_firefox/mozilla/layout/build/libgklayout.so)

And then later we reflow and render it and that causes cascading warnings in text rendering code.

In fact the root cause is probably earlier because the warnings above look like JS converting some uninitialized number to a string.

Let's figure out what the initial problem is and then we can see what's left over after that's fixed.

Comment 3

[Robert Sayre]

OK, so without the patch in 407794 applied, we're building the unit test box with "-O"... and missing a bunch of optimizations. With that patch applied, the settings are the same, which is preferable. The nightlies and releases are probably hitting this, so requesting blocking.

Comment 4

[Robert Sayre]

Attachment: warnings

These warnings in the streams code cause the bug. If we compile with -fno-strict-aliasing (and give up those optimizations), it works as expected.

Comment 5

[Boris Zbarsky [:bzbarsky]]

This might also explain the issues that the binary input streams unit tests ran into?

We might want to fix this in both, since the latter are used by fastload, which might in fact hit the bugs in question...

Comment 6

[Robert Sayre]

I wallpapered this in bug 407794, so we would stop shipping it in nightlies.

Comment 7

[Sylvain Pasche]

There might be a trick for compiling without -fno-strict-aliasing and still getting this right (I didn't test though). According to the comment, it doesn't seem to hurt too much performance wise.

http://www.google.com/codesearch/p?hl=en#h0RrPvyPu-c/base/basictypes.h&l=265

Comment 8

[Alex Vincent [:WeirdAl]]

Neil, why was this wontfixed?

Comment 9

[Neil Deakin]

scriptableIO hasn't been around for a long time, so this bug isn't needed.