Skip to content

Conversation

@disconnect3d
Copy link
Contributor

For a more detailed description, see issue #1317.

Release 4.0.0 introduced a new field for ARM operands: operand.mem.lshift. This field was supposed to be a bug fix for #246. The #246 issue has been fixed in the meantime and the proper shift value was stored in operand.shift.value.

The 4.0.0 changes created a regression in which operand.shift.value was not set for a tbh [r0, r1, lsl #1] instruction on ARM and the value was set in a operand.mem.lshift field instead.

As the regression broke some of users codebase (e.g. in manticore project), we fix it by setting operand.shift.value back again.

As a result, the shift value is set in two fields: operand.shift.value and operand.mem.lshift.

As the operand.shift also stores a .type field, we might want to deprecate operand.mem.lshift in the future.

For a more detailed description, see issue capstone-engine#1317.

Release 4.0.0 introduced a new field for ARM operands:
`operand.mem.lshift`. This field was supposed to be a bug fix for capstone-engine#246.
The capstone-engine#246 issue has been fixed in the meantime and the proper shift value
was stored in `operand.shift.value`.

The 4.0.0 changes created a regression in which `operand.shift.value`
was not set for a `tbh [r0, r1, lsl capstone-engine#1]` instruction on ARM and the
value was set in a `operand.mem.lshift` field instead.

As the regression broke some of users codebase (e.g. in
[manticore](trailofbits/manticore#1312) project), we fix it by setting
`operand.shift.value` back again.

As a result, the shift value is set in two fields: `operand.shift.value`
and `operand.mem.lshift`. As the `operand.shift` also stores a `.type`
field, we might want to deprecate `operand.mem.lshift` in the future.
@aquynh aquynh merged commit f4d701f into capstone-engine:master Jan 2, 2019
@aquynh
Copy link
Collaborator

aquynh commented Jan 2, 2019

merged, thanks!

@disconnect3d disconnect3d deleted the fix-arm-thb-shift-value branch January 8, 2019 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants