510281 - uninitialized memory access in BigInteger

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Edwin Smith]

valgrind reports:

==32034== Conditional jump or move depends on uninitialised value(s)
==32034==    at 0x812D9DE: avmplus::BigInteger::addOrSubtract(avmplus::BigInteger const*, bool, avmplus::BigInteger*) const (BigInteger.cpp:630)
==32034==    by 0x80C228E: avmplus::BigInteger::compareOffset(avmplus::BigInteger const*, avmplus::BigInteger const*) (BigInteger.h:106)
==32034==    by 0x80BFFAE: avmplus::D2A::nextDigit() (MathUtils.cpp:1781)
==32034==    by 0x80C13CE: avmplus::MathUtils::convertDoubleToString(avmplus::AvmCore*, double, int, int) (MathUtils.cpp:983)
==32034==    by 0x809B80C: avmplus::AvmCore::string(int) (AvmCore.cpp:3560)

==32034==  Uninitialised value was created by a stack allocation
==32034==    at 0x80C2254: avmplus::BigInteger::compareOffset(avmplus::BigInteger const*, avmplus::BigInteger const*) (BigInteger.h:89)
==32034== 

how to run:

1. build with AVMFEATURE_USE_SYSTEM_MALLOC=1
2. on release builds, use -O3 -g to get symbols (haven't tried debug builds)
3. get supressions file MMgc/mmgc.supp from Bug 509020
3. valgrind -q --track-origins=yes --suppressions=MMgc/mmgc.supp shell/avmshell <files>

seen in these tests:
ecma3/GlobalObject/e15_1_2_2_2.abc
ecma3/TypeConversion/e9_3_1_3_rt.abc
ecma3/LexicalConventions/e7_7_3_1.abc

Comment 1

[Brent Baker]

Confirmed that valgrind does report "Conditional jump or move depends on uninitialised value(s)" in avmplus::BigInteger when running the above testcases.

NOTE: The suppression file is out of date and you will see a lot of MMgc related messages when you run the tests.

Comment 2

[Lars T Hansen]

The error is signaled also with -O1 (so it's probably right) and is probably caused by the backward scan stepping beyond the start of the workBuffer when it's discarding leading zeroes.  This can happen if the operation is 'add' and the inputs are both zero; the test for a zero result earlier in addOrSubtract does not catch this case.

Comment 3

[Lars T Hansen]

Attachment: Patch

Add a missing check for a zero result in addOrSubtract; add an assert that will catch the problem if there are other corner cases.

Comment 4

[Lars T Hansen]

redux changeset:   2759:c0a53a3f9cb8 (also 2758)