pharo-vm icon indicating copy to clipboard operation
pharo-vm copied to clipboard

Bytecode descriptor's `isMapped` should enforce generated instructions are annotated

Open nrainhart opened this issue 2 years ago • 1 comments

Bytecodes declared in StackToRegisterMappingCogit#genMappedInlinePrimitive: can specify metadata that will be part of the bytecode descriptor. In particular, isMapped is used to indicate that the bytecode may end up calling a message send, and thus requires to keep a mapping between the bytecode program counter and the machine code one.

However, that metadata is not enough for everything to work: the generated abstract instructions also need to be annotated using Cogit#annotateBytecode:, or else things will fail unpredictably.

These two mechanisms currently need to be kept in sync manually, which makes it easy for bugs to creep in (e.g. see https://github.com/nrainhart/opensmalltalk-vm/commit/5a83ea16b7b612012dd5cbb58e7591f5397c4abc).

We should make this less error-prone, by either adding a validation or enforcing that all bytecodes annotated with isMapped automatically annotate the abstract instructions they generate with Cogit#annotateBytecode:.

cc @guillep in case you want to correct my comments or add something else.

nrainhart avatar Jun 30 '22 14:06 nrainhart

Thanks for the issue :)

El 30 jun 2022, a las 16:07, nrainhart @.***> escribió:

Bytecodes declared in StackToRegisterMappingCogit#genMappedInlinePrimitive: can specify metadata that will be part of the bytecode descriptor. In particular, isMapped is used to indicate that the bytecode may end up calling a message send, and thus requires to keep a mapping between the bytecode program counter and the machine code one.

However, that metadata is not enough for everything to work: the generated abstract instructions also need to be annotated using Cogit#annotateBytecode:, or else things will fail unpredictably.

These two mechanisms currently need to be kept in sync manually, which makes it easy for bugs to creep in (e.g. see @.*** https://github.com/nrainhart/opensmalltalk-vm/commit/5a83ea16b7b612012dd5cbb58e7591f5397c4abc).

We should make this less error-prone, by either adding a validation or enforcing that all bytecodes annotated with isMapped automatically annotate the abstract instructions they generate with Cogit#annotateBytecode:.

cc @guillep https://github.com/guillep in case you want to correct my comments or add something else.

— Reply to this email directly, view it on GitHub https://github.com/pharo-project/pharo-vm/issues/443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFM5YSRCWLANZ4SKZOIM7LVRWSY7ANCNFSM52JPZHEQ. You are receiving this because you were mentioned.

guillep avatar Jun 30 '22 14:06 guillep