596608 - _to_method_handler copies uninitialized stack data into data segment

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Tommy Reilly]

Eventually this data finds its way into a MethodInfo, the GC scans it and valgrind flags the read there.

Interestingly only happens on 64 bit builds, release and debug.

Comment 1

[Tommy Reilly]

Attachment: naive fix

There's gotta be a way to fix this that does away with _to_method_handler function.   Apparently its only needed for MSVC (the AOT approach works everywhere but).

Comment 2

[Steven Johnson]

Comment on attachment 475583 [details] [diff] [review]
naive fix

Seems reasonable, but use NULL instead of 0 (gcc 4.5 will complain otherwise).

Comment 3

[Tommy Reilly]

http://hg.mozilla.org/users/treilly_adobe.com/tr-valgrind/rev/ccd6eabee566

I thought NULL too but Edwin seemed to prefer zero, I'll never understand the nuances of that debate (by choice)

Comment 4

[Steven Johnson]

Edwin prefers zero, but gcc4.5 is forcing the issue by spewing warnings if you use 0 for a pointer type

Comment 5

[Lars T Hansen]

Although Edwin is technically correct he is morally in the wrong - NULL carries information to the reader, and 0 does not.  (Also good compilers define NULL as something other than "0" so that NULL can't be abused in non-pointer contexts.) The absence of "nil" or a similar null-pointer idiom from C is a botch, but matters more in some decades than in others.  Apparently we're on the upswing again after a decade of complacency.

Comment 6

[Tommy Reilly]

Comment on attachment 475583 [details] [diff] [review]
naive fix

Couldn't think of an easy way to remove this hack so thinking we push this and create another bug to figure out how to do away with _to_method_handler?

Comment 7

[Edwin Smith]

Comment on attachment 475583 [details] [diff] [review]
naive fix

sounds good. Create the bug and add a "fixme: bug XXX" comment before pushing.

Comment 8

[Tommy Reilly]

new bug 599931

Comment 9

[Tommy Reilly]

http://hg.mozilla.org/tamarin-redux/rev/05f2ed27e868