Subj : BUG   :(  Re: Compacting JS source via ParseTree or Decompile
To   : netscape.public.mozilla.jseng
From : Dan Libby
Date : Thu Jun 10 2004 04:43 pm

I found a nasty bug in the compactor. I know of one [untested] 
workaround, but hopefully there is a better solution.

For my main question, skip down to "QUESTION" below.  I provide the rest 
of this for background.

Bug Description:
----------------

If a function contains a locally declared variable pointing to a global 
variable, and another, later function contains the same local variable 
name pointing to the same global variable, then the compactor ends up 
using the same variable name for both local vars in both functions. 
Since the variables names are incremented from 'a' based on the number 
of local (or arg) vars per function, this has great potential to break 
things.

Example Input That Breaks:
--------------------------

globalVar = "stuff";

function checkout()
{
   var localVarRefGlobal = globalVar;
   var localVar2 = 2;
   var localVar3 = 3;
}

function betcart_Check_Balance()
{
   var localVarRefGlobal = globalVar;
}


Output:  (pretty printed)
-------------------------

globalVar = "stuff";

function checkout() {
     var a = globalVar;
     var b = 2;
     var a = 3;
}

function betcart_Check_Balance() {
     var a = globalVar;
}


Writeup:
--------

Notice that we now have local variable 'a' defined twice in the checkout 
   function.  If the 2nd function did not exist, or if it used a 
different local variable name, then we would have correctly: 'a', 'b', 
and 'c'.

Thus, it appears to me that internally these two local variables are 
really the same object.  When I change the id atom for the var in the 
latter func, it also gets updated in the former.

QUESTION:
I think that to fix this correctly, I need a way to determine if a given 
local variable is referencing a global variable.  If so, I will leave it 
alone (or possibly do some sort of clever lookup to guarantee global 
uniqueness).  I took a look in the headers, and I'm guessing that 
SPROP_IS_ALIAS and/or JSPROP_SHARED might be relevant...?  So, what is 
the "right way"?

A grosser alternative is to simply use a global variable namespace 
throughout the script, rather than the current per function namespace. 
This would have the drawback that after 'z' we wrap to 'aa', thus 
creating a larger output.

The code:
---------

void shorten_function_vars(work_plan* wp, JSContext* cx, JSFunction* fun) {
     JSScopeProperty* sp = OBJ_SCOPE(fun->object)->lastProp;
     #define NAME_LEN 6
     char var_name[NAME_LEN] = {0};

     while( sp ) {
         if( ( !wp->localvarsoff && sp->getter == js_GetLocalVariable) ||
             ( !wp->argvarsoff && sp->getter == js_GetArgument) ) {
             jsid idval = ID_TO_VALUE(sp->id);
             JSString* idstr = js_ValueToString(cx, idval);

             // increment variable name, eg: 'a', 'b', .. 'z', 'aa'
             inc_name(var_name, NAME_LEN);
             JSAtom* atom = js_Atomize(cx, var_name, strlen(var_name), 0);
             sp->id = (jsid)atom;
         }
         sp = sp->parent;
     }
}

.