migration to mozjs-52

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

migration to mozjs-52

a.nekrylau
Greetings!

I've been working on some updates to OpenVXI and decided to update all the libraries first.
Looks like I was able to figure out most of it, but still have some questions (also I should mention that this is my first big c/c++ endeavor)

I have this type of thing going on and I'm not sure what to replace it with as all/most of these functions seem to be obsolete but JSAPI User guide still has them in there.

bool jsScriptRes = JS_CompileUCScript (context,
                                   tmpscript, tmpscriptlen,
                                  options, &jsScript);
  if ( ! jsScriptRes )
    rc = VXIjsi_RESULT_SYNTAX_ERROR;
  else {
    JSObject *jsScriptObj = JS_NewScriptObject (context, jsScript);
    if (( ! jsScriptObj ) ||
        ( ! JS_AddNamedRoot (context, &jsScriptObj, SCRIPT_OBJECT_NAME) )) {
      JS_DestroyScript (context, jsScript);
      rc = VXIjsi_RESULT_OUT_OF_MEMORY;
    } else {
     
      JS::Rooted <JS::Value> val(context, JS::UndefinedValue());
      JS::Rooted <JSObject*> robj(context, currentScope->GetJsobj( ));

      if ( JS_ExecuteScript (context, jsScript,
                             &val) ) {
        if ( retval )
          rc = retval->Set (val);
      } else if ( exception ) {
        rc = VXIjsi_RESULT_SCRIPT_EXCEPTION;
      } else if ( numBranches > maxBranches ) {
        rc = VXIjsi_RESULT_SECURITY_VIOLATION;
      } else {
        rc = VXIjsi_RESULT_NON_FATAL_ERROR;
      }

      if ( ! JS_RemoveRoot (context, &jsScriptObj) )
        rc = VXIjsi_RESULT_FATAL_ERROR;
    }
  }
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: migration to mozjs-52

Jason Orendorff-2
I don't have mozjs-52 handy, but I can give you a few pointers.

You don't need a replacement for JS_NewScriptObject, JS_DestroyScript,
JS_AddNamedRoot, or JS_RemoveRoot. They are all related to GC-safety and
you shouldn't need anything more complicated than JS::Rooted for that.

Instead of separately calling JS_CompileScript and JS_ExecuteScript, you
should call JS::Evaluate (in js/public/CompilationAndEvaluation.h) if it
exists in mozjs-52. If not, I think it was called JS_EvaluateScript.

If you need a CompileOptions object, use JS::OwnedCompileOptions, defined
in js/public/CompileOptions.h, and make sure to call init() on it before
using it.

-j

On Tue, Jul 2, 2019 at 8:35 AM <[hidden email]> wrote:

> Greetings!
>
> I've been working on some updates to OpenVXI and decided to update all the
> libraries first.
> Looks like I was able to figure out most of it, but still have some
> questions (also I should mention that this is my first big c/c++ endeavor)
>
> I have this type of thing going on and I'm not sure what to replace it
> with as all/most of these functions seem to be obsolete but JSAPI User
> guide still has them in there.
>
> bool jsScriptRes = JS_CompileUCScript (context,
>                                    tmpscript, tmpscriptlen,
>                                   options, &jsScript);
>   if ( ! jsScriptRes )
>     rc = VXIjsi_RESULT_SYNTAX_ERROR;
>   else {
>     JSObject *jsScriptObj = JS_NewScriptObject (context, jsScript);
>     if (( ! jsScriptObj ) ||
>         ( ! JS_AddNamedRoot (context, &jsScriptObj, SCRIPT_OBJECT_NAME) ))
> {
>       JS_DestroyScript (context, jsScript);
>       rc = VXIjsi_RESULT_OUT_OF_MEMORY;
>     } else {
>
>       JS::Rooted <JS::Value> val(context, JS::UndefinedValue());
>       JS::Rooted <JSObject*> robj(context, currentScope->GetJsobj( ));
>
>       if ( JS_ExecuteScript (context, jsScript,
>                              &val) ) {
>         if ( retval )
>           rc = retval->Set (val);
>       } else if ( exception ) {
>         rc = VXIjsi_RESULT_SCRIPT_EXCEPTION;
>       } else if ( numBranches > maxBranches ) {
>         rc = VXIjsi_RESULT_SECURITY_VIOLATION;
>       } else {
>         rc = VXIjsi_RESULT_NON_FATAL_ERROR;
>       }
>
>       if ( ! JS_RemoveRoot (context, &jsScriptObj) )
>         rc = VXIjsi_RESULT_FATAL_ERROR;
>     }
>   }
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: migration to mozjs-52

a.nekrylau
On Tuesday, July 2, 2019 at 2:52:08 PM UTC-4, Jason Orendorff wrote:

> I don't have mozjs-52 handy, but I can give you a few pointers.
>
> You don't need a replacement for JS_NewScriptObject, JS_DestroyScript,
> JS_AddNamedRoot, or JS_RemoveRoot. They are all related to GC-safety and
> you shouldn't need anything more complicated than JS::Rooted for that.
>
> Instead of separately calling JS_CompileScript and JS_ExecuteScript, you
> should call JS::Evaluate (in js/public/CompilationAndEvaluation.h) if it
> exists in mozjs-52. If not, I think it was called JS_EvaluateScript.
>
> If you need a CompileOptions object, use JS::OwnedCompileOptions, defined
> in js/public/CompileOptions.h, and make sure to call init() on it before
> using it.
>
> -j
>
> On Tue, Jul 2, 2019 at 8:35 AM <[hidden email]> wrote:
>
> > Greetings!
> >
> > I've been working on some updates to OpenVXI and decided to update all the
> > libraries first.
> > Looks like I was able to figure out most of it, but still have some
> > questions (also I should mention that this is my first big c/c++ endeavor)
> >
> > I have this type of thing going on and I'm not sure what to replace it
> > with as all/most of these functions seem to be obsolete but JSAPI User
> > guide still has them in there.
> >
> > bool jsScriptRes = JS_CompileUCScript (context,
> >                                    tmpscript, tmpscriptlen,
> >                                   options, &jsScript);
> >   if ( ! jsScriptRes )
> >     rc = VXIjsi_RESULT_SYNTAX_ERROR;
> >   else {
> >     JSObject *jsScriptObj = JS_NewScriptObject (context, jsScript);
> >     if (( ! jsScriptObj ) ||
> >         ( ! JS_AddNamedRoot (context, &jsScriptObj, SCRIPT_OBJECT_NAME) ))
> > {
> >       JS_DestroyScript (context, jsScript);
> >       rc = VXIjsi_RESULT_OUT_OF_MEMORY;
> >     } else {
> >
> >       JS::Rooted <JS::Value> val(context, JS::UndefinedValue());
> >       JS::Rooted <JSObject*> robj(context, currentScope->GetJsobj( ));
> >
> >       if ( JS_ExecuteScript (context, jsScript,
> >                              &val) ) {
> >         if ( retval )
> >           rc = retval->Set (val);
> >       } else if ( exception ) {
> >         rc = VXIjsi_RESULT_SCRIPT_EXCEPTION;
> >       } else if ( numBranches > maxBranches ) {
> >         rc = VXIjsi_RESULT_SECURITY_VIOLATION;
> >       } else {
> >         rc = VXIjsi_RESULT_NON_FATAL_ERROR;
> >       }
> >
> >       if ( ! JS_RemoveRoot (context, &jsScriptObj) )
> >         rc = VXIjsi_RESULT_FATAL_ERROR;
> >     }
> >   }
> > _______________________________________________
> > dev-tech-js-engine mailing list
> > [hidden email]
> > https://lists.mozilla.org/listinfo/dev-tech-js-engine
> >

thank you so much, Jason!

so I basically replaced that with:

JS::CompileOptions options(context);
options.setFileAndLine(NULL, 1);

JS::Rooted <JSScript*> jsScript(context);
JS::Rooted <JS::Value> val(context, JS::UndefinedValue());

if ( JS::Evaluate(context, options, tmpscript, tmpscriptlen, &val) ) {
      if ( retval )
        rc = retval->Set (val);
} ....

gcc is not screaming at me about this part anymore :)

I also figured JS_DestroyIdArray is deprecated in 52 (the doc hasn't been updated yet apparently).. do I still need to use it to release the pointer assigned by JS_Enumerate (context, obj, &props)?

the other thing is, I've been trying to figure out how to make this expression work without JS_GetStringChars function to use it with JS_GetUCProperty later:

name = JS_GetStringChars(prName.toString());
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: migration to mozjs-52

Steve Fink-4
On 7/2/19 1:00 PM, [hidden email] wrote:
> the other thing is, I've been trying to figure out how to make this
> expression work without JS_GetStringChars function to use it with
> JS_GetUCProperty later:
> name = JS_GetStringChars(prName.toString());

That's a generally dangerous thing to be doing, since you now have a
pointer to GC-controlled data that could be freed at any time. If
AutoStableStringChars exists, that's probably the safe way to go about it.

But you should also be able to use the string "directly" through some
horrendously ugly APIs. Call |interned = JS_AtomizeAndPinJSString(cx,
prName.toString()), then |||INTERNED_STRING_TO_JSID(cx, interned) on
that, to get a jsid you can use for JS_GetProperty (error-checking all
the way).||

||https://searchfox.org/mozilla-central/source/js/src/gdb/tests/test-jsid.cpp#7-9||

||
||

||||

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: migration to mozjs-52

Steve Fink-4
Though now that I suggested that approach, I'm having second thoughts.
The "pin" portion of JS_AtomizeAndPinJSString will keep the string alive
forever. And we seem to have a hole in our API -- I don't see any way to
just atomize a JSString* without extracting its characters. (Externally,
I mean. Internally, there js::AtomizeString.) I suppose it's not often
needed? You'd normally be starting from a string literal or something.

Anyway, it's probably fine if you're not going to be using this on a
large number of distinct strings. (If you use strings with the same
contents, it will reuse the existing atom, so that's fine.) If you do
need lots of different strings, then you should probably extract the
chars and use JS_AtomizeString on them to avoid leaking. It looks like
you can use AutoCheckCannotGC and JS_GetTwoByteFlatStringChars for this,
then call JS_AtomizeUCString (see the description in jsapi.h). That
would normally make me nervous, as I would expect JS_AtomizeUCString to
be able to GC and thus mangle the string you're using. But it looks like
we make it fail and return nullptr without GCing, so it's ok.

And we should implement JS_AtomizeString (or JS::AtomizeString) for
future versions.

On 7/10/19 6:21 AM, Alex Nekrylau wrote:

> Thank you Steve, with your help I was able to make that part work!
>
>> On Jul 8, 2019, at 2:55 PM, Steve Fink <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> On 7/2/19 1:00 PM, [hidden email] wrote:
>>> the other thing is, I've been trying to figure out how to make this
>>> expression work without JS_GetStringChars function to use it with
>>> JS_GetUCProperty later:
>>> name = JS_GetStringChars(prName.toString());
>>
>> That's a generally dangerous thing to be doing, since you now have a
>> pointer to GC-controlled data that could be freed at any time. If
>> AutoStableStringChars exists, that's probably the safe way to go
>> about it.
>>
>> But you should also be able to use the string "directly" through some
>> horrendously ugly APIs. Call |interned = JS_AtomizeAndPinJSString(cx,
>> prName.toString()), then |||INTERNED_STRING_TO_JSID(cx, interned) on
>> that, to get a jsid you can use for JS_GetProperty (error-checking
>> all the way).||
>>
>> ||https://searchfox.org/mozilla-central/source/js/src/gdb/tests/test-jsid.cpp#7-9||
>>
>> ||
>> ||
>>
>> ||||
>>
>

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine