885631 - JS engine creates unaligned reference to JSObject (0x42)

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Yeuk Hon Wong]

Please refer to this MDN page:
https://developer.mozilla.org/en-US/docs/Building_SpiderMonkey_with_UBSan

Comment 1

[Jesse Ruderman]

Just starting the shell is enough to trigger this UBSan error.

> http://hg.mozilla.org/mozilla-central/annotate/7ba8c86f1a56/js/public/Value.h#l1251
> http://hg.mozilla.org/mozilla-central/annotate/7ba8c86f1a56/js/src/vm/Interpreter.h#l408

One possibility is to change it form 0x42 to 0x40, but I'm not sure even that is defined behavior, and Waldo likes the unusual number.

Waldo suggests instead adding a "backdoor" method that bypasses setObject and sets the bits to 0x42.  That would avoid creating a misaligned (and bogus) reference, which seems to be worse than creating such a pointer, given the description of -fsanitize=alignment:

"Use of a misaligned pointer or creation of a misaligned reference"

Comment 2

[Yeuk Hon Wong]

I am back to this ticket starting tonight, trying to work my way through UBSan for the remaining time of my internship.

The change require in Interpreter.h, the safe-crash pointer, has been moved to js/src/vm/objectImpl.h recently.

I grepped and the modification took place as follows (base on the tip, 136366 would be 205d42d1ea46):

vagrant@precise64:~/mozilla-central/js/src$ hg grep --all -r 265427a0694d:2268ff80683a "0x42" vm/Interpreter.h
js/src/vm/Interpreter.h:133774:+:        v->setObject(*reinterpret_cast<JSObject *>(0x42));
js/src/vm/Interpreter.h:136366:-:        v->setObject(*reinterpret_cast<JSObject *>(0x42));

Here is the changelog: http://hg.mozilla.org/mozilla-central/diff/205d42d1ea46/js/src/vm/ObjectImpl.h#l1.23

Just for the record :)

Comment 3

[Jeff Walden [:Waldo]]

It occurs to me, we could have something like

class Value
{
    ...
  public:
    void setToBogusObject() {
        asBits = build jsval tag using 0x42, without casting any JSObject stuff
    }
};

rather than doing the cast here.  Would require pushing it into Value itself, but that wouldn't be so bad, to get rid of this issue.