900998 - Static analysis is missing WorkerPrivate::Create

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

jonco noticed that the static analysis doesn't seem to know about WorkerPrivate::Create.

It *is* mentioned in callgraph.txt, but only as a callee. The full name is

static already_AddRefed<mozilla::dom::workers::WorkerPrivate> mozilla::dom::workers::WorkerPrivate::Create(JSContext*, JS::Handle<JSObject*>, mozilla::dom::workers::WorkerPrivate*, JS::Handle<JSString*>, bool)

called by

Worker.cpp:int32 {anonymous}::Worker::ConstructInternal(JSContext*, uint32, JS::Value*, uint8, JSClass*)

WorkerPrivate::Create is not a key in any of the *.xdb files. It is a static function, and shows up in the json output of the above caller. It can trigger a GC, hence the concern. Here's the portion that mentions it:

  "Exp": [
    {
    "Kind": "Var",
    "Variable": {
     "Kind": "Func",
     "Name": [
       "static already_AddRefed<mozilla::dom::workers::WorkerPrivate> mozilla::dom::workers::WorkerPrivate::Create(JSContext*, JS::Handle<JSObject*>, mozilla::dom::workers::WorkerPrivate*, JS::Handle<JSString*>, bool)",
       "Create"
      ]
     }
    },
    {
    "Kind": "Var",
    "Variable": {
     "BlockId": {
      "Kind": "Function",
      "Variable": {
       "Kind": "Func",
       "Name": [
         "Worker.cpp:int32 {anonymous}::Worker::ConstructInternal(JSContext*, uint32, JS::Value*, uint8, JSClass*)",
         "ConstructInternal"
        ]
       }
      },
     "Kind": "Temp",
     "Name": [
       "__temp_16",
       "__temp_16"
      ]
     }
    }
   ],

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: xgill log file

For posterity, this is the portion of the log file that clued bhackett into the fact that it's not handling nullptr yet.

Comment 2

[Brian Hackett [Laid off!]]

It actually turns out that nullptr constants were being handled ok, just not the nullptr type (???) and this function was missing due to some other unhandled constructs the details of which I forget or never knew (fixed this a couple weeks ago but didn't post or push anything).  I've updated the sixgill repository so that the CFG for WorkerPrivate::Create and various other functions should now be generated, mostly by making the translation more tolerant of unhandled language constructs like pointer-to-member.  The translation will not be correct in these cases but there will at least be an approximate CFG generated with uses of the bad expressions replaced by some 'error' variable.  This work isn't quite done yet, there are still about 1200 different CFGs (many are template instantiations) being dropped, and I'd like to drive this to zero.

Comment 3

[Terrence Cole [:terrence]]

Has this been fixed?

Comment 4

[Steve Fink [:sfink] [:s:]]

(In reply to Terrence Cole [:terrence] from comment #3)
> Has this been fixed?

Yes. Yes? I guess.

WorkerPrivate::Create was removed in 22c5b68ae2e9 (bug 919885), so I don't have a good test for that. I'd need to redo the analysis on an older version or something. It is no longer spitting out an error for that file, though I can't tell if it's doing the right thing for the analysis instead, or just omitting the interesting parts of the CFG from the output.

But we do have bhackett's fix in, at least.