AviSynth scripting language bug?

[maxxon]

Has anyone noticed that if you call a function that throws an exception within a try block, all variables are wiped from the stack frame? I mean the entire stack frame is just gone, even in previous function scopes all the way to the top level. But if you throw an exception within the try block without creating another function scope, everything works fine.

Globals don't seem to be affected though.

Here is an example:

global output__junk = BlankClip(height=0, width=0, length=0)
global output__filename = "output.log"
function output(string s)
{
    output__junk.WriteFileStart(output__filename, "s", append=true)
    return 1
}

global g = "GLOBAL"
l = "LOCAL"

function fail
{
    assert(false)
}

try { assert(false) } catch(e) {}
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

try { fail } catch(e) {}
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

l = "LOCAL"
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

output("fn1")
function fn1
{
    l1 = "LOCAL1"

    try { assert(false) } catch(e) {}
    try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
    try { output("l1="+l1) } catch(e) { output("l1 doesn't exist.  "+e) }
}
fn1
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

output("fn2")
function fn2
{
    l1 = "LOCAL1"

    try { fail } catch(e) {}
    try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
    try { output("l1="+l1) } catch(e) { output("l1 doesn't exist.  "+e) }
}
fn2
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

The output.log file will contain this:

g=GLOBAL
l=LOCAL
g=GLOBAL
l doesn't exist.  I don't know what "l" means
(test.avs, line 23)
g=GLOBAL
l=LOCAL
fn1
g=GLOBAL
l1=LOCAL1
g=GLOBAL
l=LOCAL
fn2
g=GLOBAL
l1 doesn't exist.  I don't know what "l1" means
(test.avs, line 49)
g=GLOBAL
l doesn't exist.  I don't know what "l" means
(test.avs, line 53)

Has anyone experienced this before? I'm using AviSynth 2.60 May 25 2011.

Strange bug,


Mx

[Gavino]

Interesting...
What is happening is that when an exception causes a jump out of the failing function, its local scope is not being popped off the stack, so the local variables of the caller are still hidden.

With this additional test case:

function fail2
{  
    l = "FAIL2"
    assert(false)
}

l = "LOCAL"
try { fail2 } catch(e) {}
try { output("g="+g) } catch(e) { output("g doesn't exist.  "+e) }
try { output("l="+l) } catch(e) { output("l doesn't exist.  "+e) }

the log produces:

g=GLOBAL
l=FAIL2

Here is the code of function ScriptFunction::Execute (script.cpp):

AVSValue ScriptFunction::Execute(AVSValue args, void* user_data, IScriptEnvironment* env) 
{
  ScriptFunction* self = (ScriptFunction*)user_data;
  env->PushContext();
  for (int i=0; i<args.ArraySize(); ++i)
    env->SetVar(self->param_names[i], args[i]);
  AVSValue result = self->body->Evaluate(env);
  env->PopContext();
  return result;
}

The fix will involve adding code to catch exceptions in the evaluation of the function body and pop the context before passing the exception on to the caller.

[maxxon]

Originally Posted by Gavino

Interesting...
What is happening is that when an exception causes a jump out of the failing function, its local scope is not being popped off the stack, so the local variables of the caller are still hidden.

Wow! Funny.

Originally Posted by Gavino

Here is the code of function ScriptFunction::Execute (script.cpp):

AVSValue ScriptFunction::Execute(AVSValue args, void* user_data, IScriptEnvironment* env) 
{
  ScriptFunction* self = (ScriptFunction*)user_data;
  env->PushContext();
  for (int i=0; i<args.ArraySize(); ++i)
    env->SetVar(self->param_names[i], args[i]);
  AVSValue result = self->body->Evaluate(env);
  env->PopContext();
  return result;
}

The fix will involve adding code to catch exceptions in the evaluation of the function body and pop the context before passing the exception on to the caller.

Sorry, not familiar with the code. Is this where the function call is taking place, between the push and pop contexts? Is an exception in the script actually throwing an exception in the code? That would seem to indicate that there is a try...catch somewhere before this call. So a potential fix would be (if I remember my C++ correctly):

AVSValue ScriptFunction::Execute(AVSValue args, void* user_data, IScriptEnvironment* env) 
{
  ScriptFunction* self = (ScriptFunction*)user_data;
  env->PushContext();
  try {
    for (int i=0; i<args.ArraySize(); ++i)
      env->SetVar(self->param_names[i], args[i]);
    AVSValue result = self->body->Evaluate(env);
  } catch (std::exception& e) { // reference, pointer, whatever is being used or whatever object base type is being used.
    env->PopContext();
    throw e;
  }
  env->PopContext();
  return result;
}

Which would result in a minimal touching of the code. Mind you, I'm pulling this out of my ass with pretty much no context of the rest of the code.


Mx

Hmmm, this forum doesn't have the plugins for code highlighting enabled.

[Gavino]

Originally Posted by maxxon

Sorry, not familiar with the code.

It's part of the Avisynth parser/interpreter.

Is this where the function call is taking place, between the push and pop contexts? Is an exception in the script actually throwing an exception in the code? That would seem to indicate that there is a try...catch somewhere before this call. So a potential fix would be (if I remember my C++ correctly):

AVSValue ScriptFunction::Execute(AVSValue args, void* user_data, IScriptEnvironment* env) 
{
  ScriptFunction* self = (ScriptFunction*)user_data;
  env->PushContext();
  try {
    for (int i=0; i<args.ArraySize(); ++i)
      env->SetVar(self->param_names[i], args[i]);
    AVSValue result = self->body->Evaluate(env);
  } catch (std::exception& e) { // reference, pointer, whatever is being used or whatever object base type is being used.
    env->PopContext();
    throw e;
  }
  env->PopContext();
  return result;
}

That's basically what I had in mind, though I would probably use

catch (...) {
    env->PopContext();
    throw;
  }

The declaration of result (now without initialisation) would also have to be moved outside the try block.

[maxxon]

Originally Posted by Gavino

It's part of the Avisynth parser/interpreter.

Yeah, I figured that much.

Originally Posted by Gavino

That's basically what I had in mind, though I would probably use

catch (...) {
    env->PopContext();
    throw;
  }

Ah yeah, forgot about how to rethrow.

Originally Posted by Gavino

The declaration of result (now without initialisation) would also have to be moved outside the try block.

I take it that AVSValue has an 'undefined' value when not initialised? Doesn't look too hard. Who is in charge of builds?


Mx

[Gavino]

Originally Posted by maxxon

I take it that AVSValue has an 'undefined' value when not initialised?

Yes, it's defined to be 'undefined', if you see what I mean.

Who is in charge of builds?

IanB - I guess he'll be along soon to comment.

[maxxon]

Cool. Am I to assume that some of the builtin functions have magic? Such as WriteFile? Seems to be able to evaluate the strings as if it were in the same scope as it was called from.


|\/|x

[Gavino]

Originally Posted by maxxon

Am I to assume that some of the builtin functions have magic? Such as WriteFile? Seems to be able to evaluate the strings as if it were in the same scope as it was called from.

Not magic, simply that it's only script language functions that start a new scope. The built-in functions (and external plugins) are written using the API, and can optionally use the PushContext and PopContext functions (the same ones used in the ScriptFunction code above) if they really need a new scope (which is rarely).

That's why writing control functions in the script language, using Eval() on a user-supplied code fragment, is not very useful, as any variables used have to be global. I was going to post something about that on your other thread once I had time to look properly at your library package.

[IanB]

Yep, as a general rule I guess all env->PushContext()'s should be try/catch protected. I'll do a pass through the source over the weekend.

[maxxon]

Originally Posted by Gavino

Not magic, simply that it's only script language functions that start a new scope. The built-in functions (and external plugins) are written using the API, and can optionally use the PushContext and PopContext functions (the same ones used in the ScriptFunction code above) if they really need a new scope (which is rarely).

Ahhh, ic.

Originally Posted by Gavino

That's why writing control functions in the script language, using Eval() on a user-supplied code fragment, is not very useful, as any variables used have to be global. I was going to post something about that on your other thread once I had time to look properly at your library package.

Hmmm, I'm not sure that I understand your meaning. Is that how it was or how it is? Perhaps I'll have better insight as to your meaning when I see your comments.


|\/|x

[maxxon]

Originally Posted by IanB

Yep, as a general rule I guess all env->PushContext()'s should be try/catch protected. I'll do a pass through the source over the weekend.

Cool. I wonder why it was never caught before? Not many people pushing the language that hard?


|\/|x