WI #2022 Put Block Relocation info in the Context#2042
WI #2022 Put Block Relocation info in the Context#2042
Conversation
| /// </summary> | ||
| public IList<IMultiBranchContext<Node, D>> SubContexts { get; internal set; } | ||
| /// <summary> | ||
| /// The Iso Cloned RElocated BlockNode Map: Map Original Block Index to their Cloned and relocated Block Index, for instance block of |
| /// The Iso Cloned RElocated BlockNode Map: Map Original Block Index to their Cloned and relocated Block Index, for instance block of | ||
| /// target Paragraph cloned and relocated for a PERFORM instruction target. | ||
| /// </summary> | ||
| internal IDictionary<int, int> IsoClonedRelocatedMap { get; set; } |
There was a problem hiding this comment.
Why the word 'Iso' ? The name ClonedBlocksIndexMap used in the builder is clearer.
| } | ||
|
|
||
| /// <summary> | ||
| /// <inheritdoc/> |
There was a problem hiding this comment.
I'm not familiar with<inheritdoc/> but it seems it is meant to replace the whole <summary> block. Also it is useful when generating the doc but doesn't help here.
There was a problem hiding this comment.
| public int GetRelocatedBlockIndex(int initialIndex) | ||
| { | ||
| if (this.IsoClonedRelocatedMap != null) | ||
| { | ||
| if (this.IsoClonedRelocatedMap.TryGetValue(initialIndex, out var duplicateIndex)) | ||
| { | ||
| return duplicateIndex; | ||
| } | ||
| } | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Ok so how does the client know that it has to use the relocated index ? The problem is that when using the CFG, the blocks are correct (they already have been replaced by their relocated counterpart) and the client does not have to know about 'bad' blocks vs 'correct' ones. Here, when using the context it is different.
We should try to find a way to hide this notion of relocated blocks from outside of the builder.
There was a problem hiding this comment.
The issue here is that as you know, is to allow abstract interpretation to redirect itself to relocated block rather than original block that have been cloned. What we can do is to add a new method GetOriginalRelocateBlockIndexes so the client can query what blocks have been relocated inside this context.
There was a problem hiding this comment.
After building, does the original block still has a meaning ? In my understanding, original blocks are no longer connected in the graph so they should never be used or exposed after building. We should not put the burden of choosing between original blocks and relocated blocks on the client.
If it is too complex to rewrite the context on-the-fly during the build, is it possible to hide the original-to-relocated map and make the different properties of the context object do the work for the client ?
There was a problem hiding this comment.
No it is complex to clone a MultiBranchContext for each relocation and it uses space, this is a ready to use method, that I tested and it works for the abstract interpretation purpose, the client just have to know what it is about, this approach it quite simple and it works, no need to find different approach.
There was a problem hiding this comment.
The only need her is to make accessible the relocation block information that have been used during relocation, thus there is only one point to take in account is to clone the MultiBanchContextObject for direct attribute not for relocation.
trafico-bot
bot
added
⚠️ Changes requested
trafico-bot
bot
added
🔍 Ready for Review
|
So for me it's ok I just need GetRelocatedBlockIndex to do the job without changing how MultiBranchContext has been constructed. MultiBranchContext is used only in the context of abstract interpretation, documenting the purpose of these methods is enough. If the client wants to know relocated block indexes inside this Context he can call GetOriginalRelocatedBlockIndexes. |
|
|
||
| if (clonedBlock.Context != null) | ||
| { | ||
| clonedBlock.Context = (IMultiBranchContext<Node, D>)((MultiBranchContext)clonedBlock.Context).Clone(); |
There was a problem hiding this comment.
Shouldn't this be included in the Clone method of the block ?
2 participants
No description provided.