Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3937)

Issue 6353100: i#842: Fix 64-bit inc/dec encoding.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 9 months ago by yangt
Modified:
5 months, 2 weeks ago
Reviewers:
bruening
CC:
dynamorio-devs_googlegroups.com
Visibility:
Public.

Description

Reviewer: bruening@google.com i#842: Fix 64-bit inc/dec encoding. Fixes issue 842. Fix 64-bit inc/dec encoding. Not sure if this is an elegant way to fix the issue, since opcodes 40+ in 64-bit mode are 'N.E.' instead of 'Invalid' in Intel's manual.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -16 lines) Patch
M trunk/core/x86/decode_table.c View 1 chunk +16 lines, -16 lines 1 comment Download
M trunk/suite/tests/api/ir.c View 2 chunks +12 lines, -0 lines 0 comments Download

Messages

Total messages: 2
yangt
11 years, 9 months ago (2012-07-13 18:26:47 UTC) #1
bruening
11 years, 9 months ago (2012-07-13 18:49:08 UTC) #2
LGTM

http://codereview.appspot.com/6353100/diff/1/trunk/core/x86/decode_table.c
File trunk/core/x86/decode_table.c (right):

http://codereview.appspot.com/6353100/diff/1/trunk/core/x86/decode_table.c#ne...
trunk/core/x86/decode_table.c:4326: {OP_inc,  0x400000, "inc", zAX, xx, zAX, xx,
xx, i64, (fW6&(~fWC)), t64e[1][0]},
using i64 when only this encoding is invalid seems fine (this is re: your N.E.
vs Invalid question) as this is an entry just for this encoding and on decode it
will not be examined.

for efficiency, it would be nice to not have to walk 8 invalid templates every
single time.  if 64-bit mode were static it would be an easy fix.  since it's
dynamic it would require extra code and it doesn't seem like something
worthwhile is feasible.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b