822941 - IonMonkey: Valgrind detects "Conditional jump or move depends on uninitialised value(s)" with js::detail::BumpChunk::new or js::LifoAlloc::getOrCreateChunk on the stack

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

function f() {
    var x = 500
    x = x * x
    x = x * x
    return x
}
f()

shows Valgrind error messages pointing to uninitialised value being created by a heap allocation at js::detail::BumpChunk::new with a js opt shell on m-c changeset 9de611848111 with --ion-eager.

See the attached stack.

s-s because this is an uninitialised value being detected by Valgrind, but feel free to open up if this is not s-s.

Will try to autoBisect this..

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I used the following Valgrind commands:

valgrind --smc-check=all-non-file --track-origins=yes ./js --ion-eager testcase.js

This was found after running jit-tests against Valgrind. My mozconfig:

$ cat .mozconfig
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-ff-64-opt-mc-basedOn-9de611848111
mk_add_options MOZ_MAKE_FLAGS="-s"
ac_add_options --disable-debug
ac_add_options --enable-optimize="-g -O -freorder-blocks"
ac_add_options --enable-tests
#ac_add_options --with-ccache
ac_add_options --enable-valgrind
ac_add_options --disable-jemalloc

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   114092:89e5db8cf62f
user:        Brian Hackett
date:        Fri Nov 23 23:23:03 2012 -0500
summary:     Add symbolic range analysis for loop induction variables, bug 766592. r=mjrosenb

Brian, is this a possible regressor?

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: another stack

This alternative stack indicates that the uninitialised value was created by a heap allocation at malloc at js::LifoAlloc::getOrCreateChunk instead.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   123049:6a930768eb82
user:        Nicolas B. Pierron
date:        Mon Feb 25 15:41:06 2013 -0800
summary:     Bug 841666 - Use exponent over-estimation to truncate operations. r=h4writer

Nicolas, is bug 841666 a possible fix?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #4)
> autoBisect shows this is probably related to the following changeset:
> 
> The first good revision is:
> changeset:   123049:6a930768eb82
> user:        Nicolas B. Pierron
> date:        Mon Feb 25 15:41:06 2013 -0800
> summary:     Bug 841666 - Use exponent over-estimation to truncate
> operations. r=h4writer
> 
> Nicolas, is bug 841666 a possible fix?

Yes. I guess the problem was that  Range(int64_t l, int64_t h)  was not always initializing lower_infinite_ and upper_infinite_, based on the value given as input.  Even if I don't know where the *_infinite value is used in the previous code.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Assuming fixed by bug 841666 as per comment 5 then.