Add-on bustage numbers and possible mitigations for top-level block scope changes

Kris Maglione kmaglione at mozilla.com
Wed Oct 21 20:37:49 UTC 2015


Hi all,

I've been doing some analysis of add-ons hosted on AMO for code 
that will break due to https://bugzil.la/1202902. The numbers 
aren't quite as bad as I expected, but they're still not great.

If I include tests for block-scoped bootstrap methods, I see 938 
affected add-ons, affecting ~5.7 million average daily users 
(meaning the total number of affected users is at least 30% 
higher). Mossop has an in-product fix for that issue, so leaving 
it out, I still see around 180 add-ons affected, with more than 
4M ADU.

There are definitely a few false positives in my results, but 
not many. I expect there are more false negatives. The full 
details are at the end of the email.


As far as mitigations go, I think our best option is to do a 
mass re-pack of affected public add-ons on AMO, and replace all 
top-level `let` and `const` declarations with `var` 
declarations. This sounds pretty dangerous, but I think it can 
be done safely. Prior to this change, Spidermonkey actually 
rewrote top-level let declarations to var declarations at parse 
time, so the semantics of those will be exactly the same. 
Changing `const` declarations would make those properties 
writable, which is unfortunate, but should still be fairly safe.

If we use the Spidermonkey JS shell, and its Reflect API, to 
parse the scripts, we can be 100% certain that our changes are 
correct. I'm proposing that we parse the scripts, use the AST to 
find top-level let and const declarations, change them to var 
declarations, re-parse the new tree, and make sure it's 
identical, aside from the changed variable declaration types.


The other option is to simply contact the affected developers 
and ask them to update their add-ons. Ideally, we should provide 
them with an XPI with the above automated changes, that they can 
upload if they choose.

At the moment, I'm leaning strongly toward an automated repack 
of all add-ons that use top-level let or const declarations. I 
think it's likely to fix a lot more add-ons, and save developers 
a lot of time. It would also give us a framework to do more of 
these kinds of mass repacks in the future, if it goes well.


Numbers:

I broke down the numbers in three ways: by number of add-ons 
affected, by total number of instances in all add-ons, and by 
total number of average-daily-users affected. The 'all' numbers 
exclude bootstrap methods, since we're likely to have a fix for 
that soon.

The items in each breakdown are:

 * bootstrap_method - A bootstrap method in a bootstrap.js file 
   declared with block scope.

 * exported - A top-level variable declared with block scope, 
   which is later accessed as a property on a scope returned by 
   Cu.import, or a scope modified by loadSubScript.

 * imported - The converse of the above. Any name imported from a 
   Cu.import or loadSubScript scope. E.g.,

   foo.jsm:

   let fooBar = () => baz;

   foo.js:

   const { fooBar } = Cu.import("foo.jsm");

   bar.js:

   Services.scriptLoader.loadSubScript("foo.js");
   fooBar();

   There are some false positives in this result set, but I 
   looked through the results, and it's overwhelmingly accurate. 
   The false positives are mainly in the form of:

   foo.jsm:

   function foo() {}

   bar.jsm:

   const { foo } = Cu.import("foo.jsm");

   baz.jsm:

   const { foo } = Cu.import("foo.jsm");

   where the original was declared with non-block scope, and the 
   imported versions were declared with let or const.

   There will also be a lot of false negatives, for instance:

   function module(uri) { return Cu.import(uri, {}) };

   const { fooBar } = module("foobar.jsm");

   which is not an uncommon pattern.

 * shadow - A top-level block-scoped binding shadowing a binding 
   on the global. Most of these are actual temporal dead zone 
   violations, e.g.:
   
     function thing() { return THING };
     const THING = 'x';

  But a few are variables declared with let, and then accessed 
  as properties of the global.

 * shared - Top-level variables declared with `let`, in scripts 
   loaded as overlays, into a shared scope. This test is fairly 
   limited. I didn't do any checking to see if the scripts were 
   accessed by other scripts, and it doesn't apply to scripts 
   loaded into windows by any means other than <script> nodes with 
   src attributes, in registered overlays.

   There are a lot of other places this could be a problem, 
   namely scripts loaded as frame scripts, multiple scripts 
   loaded into an extension-created document, scripts loaded via 
   inline <script> tags...




Addons:

  bootstrap_method: 757
  all: 181
  imported: 91
  exported: 91
  shared: 80
  shadow: 11

Users:

  all: 4,075,486
  imported: 3,691,848
  exported: 3,691,848
  bootstrap_method: 1,633,336
  shared: 252,630
  shadow: 156,195

Instances:

  bootstrap_method: 3,076
  all: 941
  shared: 333
  exported: 307
  imported: 288
  shadow: 13

Total add-ons in sample: 18,420



-- 
Kris Maglione
Firefox Add-ons Engineer
Mozilla Corporation

The world looks with some awe upon a man who appears unconcernedly
indifferent to home, money, comfort, rank, or even power and fame.
The world feels not without a certain apprehension, that here is
someone outside its jurisdiction; someone before whom its allurements
may be spread in vain; someone strangely enfranchised, untamed,
untrammelled by convention, moving independent of the ordinary
currents of human action.
	--Winston Churchill




More information about the Dev-addons mailing list