uninitialized memory access in BigInteger

VERIFIED FIXED in flash10.1

Status

P2
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: edwsmith, Assigned: lhansen)

Tracking

unspecified
flash10.1
x86
Linux
Bug Flags:
flashplayer-qrb +
flashplayer-triage +

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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
(Reporter)

Updated

9 years ago
Flags: flashplayer-triage?

Comment 1

9 years ago
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.
Flags: flashplayer-triage?
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
(Assignee)

Updated

9 years ago
Priority: -- → P3
Target Milestone: --- → flash10.1

Updated

9 years ago
Flags: flashplayer-qrb? → flashplayer-qrb+
(Assignee)

Updated

9 years ago
Assignee: nobody → lhansen
(Assignee)

Comment 2

9 years ago
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.
Status: NEW → ASSIGNED
(Assignee)

Updated

9 years ago
Blocks: 521196
(Assignee)

Comment 3

9 years ago
Created attachment 405733 [details] [diff] [review]
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.
Attachment #405733 - Flags: review?(stejohns)
(Assignee)

Updated

9 years ago
Priority: P3 → P2

Updated

9 years ago
Attachment #405733 - Flags: review?(stejohns) → review+
(Assignee)

Comment 4

9 years ago
redux changeset:   2759:c0a53a3f9cb8 (also 2758)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Status: RESOLVED → VERIFIED

Updated

9 years ago
Status: VERIFIED → RESOLVED
Last Resolved: 9 years ago9 years ago

Updated

9 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.