-
1. Re: Function returning array
adinn Jul 24, 2012 4:45 AM (in response to mitomutant)Hi Luis,
Luis Gasca wrote:
Hi everybody,
. . .
RULE Monitor MO requests
CLASS com.i3g.Sirocco20SOAPImpl
METHOD getMOS(java.lang.String,java.lang.String,int)
AT EXIT
IF TRUE
DO traceln($!.getItem().length);
ENDRULE
"getItem" is an instance method that returns an array of Objects. Here I am getting "org.jboss.byteman.rule.exception.TypeException: FieldExpresssion.typeCheck : invalid field reference length file ..\res\bm\SOAPGwStats.btm"
I have been expecting this bug report for a while so congratulations on being the first to report it!
The problem is fairly obvious. Although an expression of the form arrayExpression.length looks like a field access it is actually a special case. I just have not yet implemented this special case in the rule type checker and interpreter/compiler. All that is needed is to extend the typecheck, interpret and compile methods on class FieldExpression to detect cases where the target of the field access has an array type and do something relevant to that case.
Could you please click on the Issue Tracker link at the top of this page and raise a JIRA for this issue (use the forum reference field to link it to this dicsussion). I will provide a fix in the next release which will probably happen within a couple of weeks.
Of course, if you are interested and feeling adventurous you could try patching the code yourself (or, indeed, anyone else reading this thread :-). Contributions are always welcome.
regards,
Andrew Dinn
-
2. Re: Function returning array
mitomutant Jul 24, 2012 4:55 AM (in response to adinn)Hi Andrew,
JIRA raised. I will take a look at the source code to see if I can patch it by myself.
Regards,
Luis
-
3. Re: Function returning array
adinn Jul 24, 2012 5:07 AM (in response to mitomutant)Hi Luis,
Luis Gasca wrote:
Hi Andrew,
JIRA raised. I will take a look at the source code to see if I can patch it by myself.
Thanks for the JIRA. Enjoy the code!
-
4. Re: Function returning array
adinn Jul 25, 2012 6:16 AM (in response to adinn)Hi Luis,
I had a look at this and realised that the fix is a little more tricky than it originally looks. Also, when writing a test case for this issue I found an error in the parser which also needs fixing. So, anyway given that resolving these two problems woudl be quite a challenge for someone new to Byteman I decided to provide a fix for this myself. In case you have been looking at the code and are interested in the details they are described below (this also serves to document the fix for the JIRA).
Firstly, the type checker fix is made more complicated by the fact that the needs to detect an attempt to use arrayExpression.length as both an LVALUE and an RVALUE i.e. as both the target of an assignment (arrayExpression.length = numericExpression) and as a normal expression yielding a value (that includes when it appears as the right hand side of an assignment but also any other location which is not the target of an assignment). Updating an array length is illegal. So, when the type checker identifies that the length field belomgs to an array type it needs to also check whether the expression is being used as an LVALUE or RVALUE.
Now, FieldExpression already knows how to operate as both an LVALUE and an RVALUE -- it is one of several subclasses of AssignableExpression which provide this extra behaviour. So, for example, it implements two methods used when executing a rule via the rule interpreter, interpret() and interpretAssign(). The former reads and returns the value stored in a field of the owner object whereas the latter writes the owner's field with a supplied value. However, currently there is only one typecheck method for all Expression classes, which is used whether the expression appears as an LVALUE or RVALUE. That's because up to now it has made no difference to the type checking algorithm where the expression appears - the type of the expression is the same whether it identifies somethign to be evaluated or assigned.
However, this fix changes that condition. FieldExpression.typecheck() needs to operate differently depending upon whether the length field of an array appears as an LVALUE or RVALUE. Not because it must compute a different type but because it has to throw a TypeException if the expression is an array length expression and appears as an LVALUE. Unfortunately, the fix requires modifying the expression tree for all expressions which can appear as LVALUES. So, all subtypes of AssignableExpression now need to implement two typecheck methods: typecheck(), which is called when the expression appears as an RVALUE; and typeCheckAssign(), which is called when the expression appears as an LVALUE. The typecheck() method for AssignmentExpression is the place where this new method gets called. Instances of AssignmentExpression typecheck their right hand expression by calling typecheck() and their left hand expression by calling typeCheckAssign().
Ok, so the second problem is that the parser needed an extra rule. I modified the test code in TestArray.java and the test rule in TestArray.btm to try to reproduce your bug and happened to use an expression like this in the rule
oarray[0].length
where oarray is of type Object[][]. The problem is that the parser was happy to acccept terms like foo.length or even this.foo().length but it was not happy to accept an array subscripted expression as the target of a field access.
This is not just a problem for nested arrays as in the example above. If we had a class Foo with field myField and an array fooArray of type Foo[] then the rule parser would not accept an expression of the form
fooArray[0].myField
The problem is in this parser rule (taken from file ECAGrammar.cup) which says what sorts of complex expression can appear on the left hand side of a field access without requiring brackets around them
expr_field_expr
::= simple_expr:se DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, se); :}
| meth_expr:me DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, me); :}
| expr_field_expr:efe DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, efe); :}
;
The fix is straightforward
expr_field_expr
::= simple_expr:se DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, se); :}
| meth_expr:me DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, me); :}
| array_expr:ae DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, ae); :}
| expr_field_expr:efe DOT simple_name:f
{: RESULT = node(ParseNode.FIELD, fleft, fright, f, efe); :}
;
Well, if you read this far I hope you understood and enjoyed the explanation. If not no problem.
regards,
Andrew Dinn
-
5. Re: Function returning array
mitomutant Jul 25, 2012 7:03 AM (in response to adinn)Hi Andrew,
I was trying to patch the code - starting with FieldExpression.typecheck() - but reading your explanation makes clear that my approach was way too simplistic.
In the meantine, I have frozen this particular rule and we are now adding additional rules for the rest our code. We are using byteman to collect some stats and dispatch them to a statsd server via a helper class. I must say that byteman is a really impressive piece of software. Being able to collect application metris without touching our source code is a game changer for us.
regards,
Luis
-
6. Re: Function returning array
adinn Jul 25, 2012 7:34 AM (in response to mitomutant)Hi Luis,
Luis Gasca wrote:
I was trying to patch the code - starting with FieldExpression.typecheck() - but reading your explanation makes clear that my approach was way too simplistic.
In the meantine, I have frozen this particular rule and we are now adding additional rules for the rest our code. We are using byteman to collect some stats and dispatch them to a statsd server via a helper class. I must say that byteman is a really impressive piece of software. Being able to collect application metris without touching our source code is a game changer for us.
Well keep an eye out for a patch release in the next few weeks.
Also, thank you for explaining what you are doign with Byteman and for the positive coments. Ad hoc measurement is one of the really nice things Byteman makes easy. If you are able share any of your code which might be useful to others as an example of how to use Byteman (such as your helper class or maybe some of your stats gathering rules) please post a link to a repo where others can access it.
regards,
Andrew Dinn
-
7. Re: Function returning array
mitomutant Jul 25, 2012 8:21 AM (in response to adinn)Just downloaded latest repo. It now works like a charm.
Sure, once I deploy everything I will be more than glad to share our helpers and rules.
regards,
Luis
-
8. Re: Function returning array
adinn Jul 25, 2012 8:59 AM (in response to mitomutant)Hi Luis,
Luis Gasca wrote:
Just downloaded latest repo. It now works like a charm.
Excellent.
Luis Gasca wrote:
Just downloaded latest repo. It now works like a charm.
Sure, once I deploy everything I will be more than glad to share our helpers and rules.
Thanks very much. I am sure it will be of interest to other Byteman users.