Courgette: Extend pointer detection in x64.

Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674
Committed: https://crrev.com/1560d9dbd68658af6659a0bdc7519eb9d18a3a2d
Cr-Commit-Position: refs/heads/master@{#402517}

[etiennep, 2016-06-17 17:25:07]

etiennep@google.com changed reviewers:
+ huangs@chromium.org

[etiennep, 2016-06-17 17:25:08]

PTAL

[huangs, 2016-06-17 18:05:51]

In terms of code, just nit on comments.

Should run more experiments to justify that 1% is not noise.  Also, are all 4
new instructions helpful?  Can try this by taking one out and see if things get
worse, etc.

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
File courgette/rel32_finder_x64.cc (right):

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
courgette/rel32_finder_x64.cc:62: p[0] == 0x8D) &&          // MOV / LEA
I think this is sufficient complex to warrant more detailed high-level comments:
// 6-byte instructions:
//   FF 15 [XXXXXXXX] : call QWORD PTR [rip+XXXXXXXX]
//   FF 25 [XXXXXXXX] : jmp  QWORD PTR [rip+XXXXXXXX]
//   89 [05|0D|...|3D] [XXXXXXXX]: mov DWORD PTR [rip+XXXXXXXX],reg
//   8B [05|0D|...|3D] [XXXXXXXX]: mov reg,DWORD PTR [rip+XXXXXXXX]
//   8D [05|0D|...|3D] [XXXXXXXX]: lea reg,[rip+XXXXXXXX]
// [05|0D|...|3D] selects reg = [eax|ecx|...|edi].
// We filter these by (p[1] & 0xC7) == 0x05.

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
courgette/rel32_finder_x64.cc:69: if (p + 7 <= end_pointer) {
// 7-byte instructions:
//   [48/4C] 89 [05|0D|...|3D] [XXXXXXXX]: mov QWORD PTR [rip+XXXXXXXX],reg
//   [48/4C] 8B [05|0D|...|3D] [XXXXXXXX]: mov reg,QWORD PTR [rip+XXXXXXXX]
//   [48/4C] 8D [05|0D|...|3D] [XXXXXXXX]: lea reg,[rip+XXXXXXXX]
// Prefix 48: [05|0D|...|3D] selects reg = [rax|rcx|...|rdi].
// Prefix 4C: [05|0D|...|3D] selects reg = [r9|r10|...|r15].

[etiennep, 2016-06-27 19:20:39]

The CQ bit was checked by etiennep@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 19:21:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[etiennep, 2016-06-27 19:27:42]

Description was changed from

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=
==========

to

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=
==========

[etiennep, 2016-06-27 19:30:24]

Description was changed from

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=
==========

to

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674
==========

[etiennep, 2016-06-27 19:41:21]

PTAnL

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
File courgette/rel32_finder_x64.cc (right):

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
courgette/rel32_finder_x64.cc:62: p[0] == 0x8D) &&          // MOV / LEA
On 2016/06/17 18:05:51, huangs wrote:
> I think this is sufficient complex to warrant more detailed high-level
comments:
> // 6-byte instructions:
> //   FF 15 [XXXXXXXX] : call QWORD PTR [rip+XXXXXXXX]
> //   FF 25 [XXXXXXXX] : jmp  QWORD PTR [rip+XXXXXXXX]
> //   89 [05|0D|...|3D] [XXXXXXXX]: mov DWORD PTR [rip+XXXXXXXX],reg
> //   8B [05|0D|...|3D] [XXXXXXXX]: mov reg,DWORD PTR [rip+XXXXXXXX]
> //   8D [05|0D|...|3D] [XXXXXXXX]: lea reg,[rip+XXXXXXXX]
> // [05|0D|...|3D] selects reg = [eax|ecx|...|edi].
> // We filter these by (p[1] & 0xC7) == 0x05.

Done.

https://chromiumcodereview.appspot.com/2072093003/diff/1/courgette/rel32_find...
courgette/rel32_finder_x64.cc:69: if (p + 7 <= end_pointer) {
On 2016/06/17 18:05:51, huangs wrote:
> // 7-byte instructions:
> //   [48/4C] 89 [05|0D|...|3D] [XXXXXXXX]: mov QWORD PTR [rip+XXXXXXXX],reg
> //   [48/4C] 8B [05|0D|...|3D] [XXXXXXXX]: mov reg,QWORD PTR [rip+XXXXXXXX]
> //   [48/4C] 8D [05|0D|...|3D] [XXXXXXXX]: lea reg,[rip+XXXXXXXX]
> // Prefix 48: [05|0D|...|3D] selects reg = [rax|rcx|...|rdi].
> // Prefix 4C: [05|0D|...|3D] selects reg = [r9|r10|...|r15].

Done.

[huangs, 2016-06-27 20:25:14]

LGTM with NITs.

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
File courgette/rel32_finder_x64.cc (right):

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:75: //   M(Mod & r/m): 00 & 101 = disp32
Think it's better to be explicit:
//  MM = 00 & MMM = 101 => rip+disp32
//  RRR selects reg operand from [eax|ecx|...|edi]

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:81: if (p + 7 <= end_pointer) {
// TODO(huangs): Maybe skip checking prefixes, and let 6-byte instructions take
care of this?

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:97: //   M(Mod & r/m): 00 & 101 = disp32
//  MM = 00 & MMM = 101 => rip+disp32
//  RRR selects reg operand from [r8|r9|...|r15]

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:108: //   M(Mod & r/m): 00 & 101 = disp32
//  MM = 00 & MMM = 101 => rip+disp32
//  RRR selects reg operand from [ax|cx|...|di]

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/testda...
File courgette/testdata/rel32_x64_02.txt (right):

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/testda...
courgette/testdata/rel32_x64_02.txt:69: 004010C9: 5D                 pop       
ebp
NIT: Spacing before "ebp".

[etiennep, 2016-06-27 21:17:39]

PTAnL

[etiennep, 2016-06-28 17:38:27]

Voila!

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
File courgette/rel32_finder_x64.cc (right):

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:75: //   M(Mod & r/m): 00 & 101 = disp32
On 2016/06/27 20:25:14, huangs wrote:
> Think it's better to be explicit:
> //  MM = 00 & MMM = 101 => rip+disp32
> //  RRR selects reg operand from [eax|ecx|...|edi]

Done.

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:81: if (p + 7 <= end_pointer) {
On 2016/06/27 20:25:14, huangs wrote:
> // TODO(huangs): Maybe skip checking prefixes, and let 6-byte instructions
take
> care of this?

Done.

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:97: //   M(Mod & r/m): 00 & 101 = disp32
On 2016/06/27 20:25:14, huangs wrote:
> //  MM = 00 & MMM = 101 => rip+disp32
> //  RRR selects reg operand from [r8|r9|...|r15]

Done.

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/rel32_...
courgette/rel32_finder_x64.cc:108: //   M(Mod & r/m): 00 & 101 = disp32
On 2016/06/27 20:25:14, huangs wrote:
> //  MM = 00 & MMM = 101 => rip+disp32
> //  RRR selects reg operand from [ax|cx|...|di]

Done.

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/testda...
File courgette/testdata/rel32_x64_02.txt (right):

https://chromiumcodereview.appspot.com/2072093003/diff/20001/courgette/testda...
courgette/testdata/rel32_x64_02.txt:69: 004010C9: 5D                 pop       
ebp
On 2016/06/27 20:25:14, huangs wrote:
> NIT: Spacing before "ebp".

Done.

[etiennep, 2016-06-28 18:40:12]

The CQ bit was checked by etiennep@google.com

[etiennep, 2016-06-28 18:40:12]

The patchset sent to the CQ was uploaded after l-g-t-m from huangs@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2072093003/#ps60001
(title: "Correct Nits in comments")

[commit-bot: I haz the power, 2016-06-28 18:41:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 20:17:35]

Description was changed from

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674
==========

to

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674
==========

[commit-bot: I haz the power, 2016-06-28 20:17:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-28 20:19:41]

Description was changed from

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674
==========

to

==========
Courgette: Extend pointer detection in x64.

These new instructions are now recognised in x64 RelFinder :
MOV QWORD[rip+offset], reg
MOV reg, DWORD[rip+offset]
MOV DWORD[rip+offset], reg
LEA reg, DWORD[rip+offset]
MOV reg, WORD[rip+offset]
MOV WORD[rip+offset], reg
LEA reg, WORD[rip+offset]

This reduces patch size by ~1% for x64 image files.

BUG=623674

Committed: https://crrev.com/1560d9dbd68658af6659a0bdc7519eb9d18a3a2d
Cr-Commit-Position: refs/heads/master@{#402517}
==========

[commit-bot: I haz the power, 2016-06-28 20:19:43]

Patchset 4 (id:??) landed as
https://crrev.com/1560d9dbd68658af6659a0bdc7519eb9d18a3a2d
Cr-Commit-Position: refs/heads/master@{#402517}