[jira] [Commented] (BCEL-295) Incorrect live range information in LocalVariableGen

["Gary Gregory (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/BCEL-295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16199046#comment-16199046 ] 

Gary Gregory commented on BCEL-295:
-----------------------------------

The patch is small and seems OK at first glance but I am not comfortable applying without a matching unit test. I do appreciate the low/no impact WRT compatibility. That's a plus :-) What do others think?

> Incorrect live range information in LocalVariableGen
> ----------------------------------------------------
>
>                 Key: BCEL-295
>                 URL: https://issues.apache.org/jira/browse/BCEL-295
>             Project: Commons BCEL
>          Issue Type: Bug
>            Reporter: Mark Roberts
>         Attachments: LocalVariableGen.diff
>
>
> (Not sure of priority - blocker for me, but probably of little consequence for most clients.)
> There is a design flaw with the treatment of local variable live ranges.  In the .class file these are demarcated via byte code offsets into the method's instructions.  The range is from start up to, but not including, the length. If the live range lasts through the end of the method, then length points to the first byte 'past' the end of the method.  BCEL converts these offsets into InstructionHandles and in doing so can no longer differentiate between a live range that ends prior to the last instruction of the method or one that includes the last instruction of the method.
> How to fix this is a bit of a problem.  The 'correct' solution would seem to be to keep end as a null InstructionHandle as indicated by some of the comments.  Unfortunately, LocalVariableGen does not have access to the method's InstructionList and thus cannot easily convert this null pointer back to the correct length for output.  We could grab the InstructionHandle for start and then count the instruction bytes as we iterate to the end.
> But all this still begs the question of the fact this would be a change in behavior - a client would now have to check for a null end handle before dereferencing.  The proposed fix I have attached takes another approach.  It sets a flag in the constructor if the initial value for end is null and then lets all else proceed unchanged.  A client may test this flag to see if the special case is active.  Also, the getLocalVariable method uses this flag to correctly set the length on output.  
> I believe this approach would have no effect on existing code.  We would only need to document the new flag for clients that might care.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)