634166 - valgrind should detect reading off the end of argv

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

We've had examples where code forgot to check argc before reading a slot from argv. In these cases, anything can happen, from finding a "double" to finding an old object that happened to have been left on the stack. It appears that because the memory that we pass to native functions is just in the middle of the contiguous stack, it's likely to be in the middle of a block of memory that is otherwise "allocated" but hasn't been set-up for the function call.

Would it be possible to add annotations or something so that valgrind would see these reads as being reading uninitialized memory?

dvander also suggested that this might be something to throw static analysis at instead of valgrind.

Comment 1

[Nicholas Nethercote [inactive]]

You could mark the memory just after argv as inaccessible with Valgrind, yes.  Question is, what is that memory?  Is it something that's accessed?  If so, that'll lead to false positive errors.

Comment 2

[Luke Wagner [:luke]]

At all times there is an exact top-of-stack (in the general case, evaluated by StackSpace::firstUnused).  Usually its just cx->regs->sp.  Rather than trying to notify valgrind on every push/pop (hard), perhaps we could have a mechanism for telling valgrind the address of a variable which acts as the top of a stack?  That way there would only be a handful of places to update; mostly StackSpace/JSContext memfuns.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

How worried are we about the interpreter/a JIT reading off the end of the stack? Do you think it'd be worth simply marking the end of the stack when calling into natives/getters/setters and then expanding the end of the stack when we return?