Add support for RIP relative addresses on x86_64.

Add support for RIP relative addresses on x86_64.

Visual studio uses rip-relative addressing (rra) extensively in 64 bit binaries. ParseRel32RelocsFromSection does not find these addresses, which causes courgette to miss a lot of rva pointers, and thus missing a lot of compresssion opportunities.

This patch makes the ParseRel32RelocsFromSection find rip relative call/jmp/lea. It also finds mov's that load from memory using rra's.

Based on CL by niels.werensteijn.teamspeak@gmail.com in crrev.com/212563003

This change gives a noticeable improvement on 64-bit binaries. Against test binaries (64-bit chrome.dll 40.0.2214.115->43.0.2317.0) the patch sizes were:

Uncompressed:
before: 10,948,152
after: 9,948,442 (9.1% reduction)

Compressed (7z ultra):
before: 6,084,670
after: 5,581,502 (8.3% reduction)

BUG=459064
TEST=courgette_unittests

Committed: https://crrev.com/4f4f19d78bb91ba7af7e79f86b75d1f67aa1caf6
Cr-Commit-Position: refs/heads/master@{#321524}

[Will Harris, 2015-02-28 20:49:58]

wfh@chromium.org changed reviewers:
+ dgarrett@chromium.org

[Will Harris, 2015-02-28 21:05:57]

CL necro the CL that Niels originally wrote. PTAL.

[Will Harris, 2015-03-05 17:51:00]

ping dgarrett@

[Will Harris, 2015-03-09 23:02:15]

wfh@chromium.org changed reviewers:
+ rickyz@chromium.org

[Will Harris, 2015-03-09 23:02:34]

On 2015/03/05 17:51:00, Will Harris wrote:
> ping dgarrett@

rickyz says he will take a look at this assembly parsing.

[dgarrett, 2015-03-10 03:03:48]

This generally looks good, though I have no idea about the assembly parsing.
Deferring to rickyz, based on assembly expertise.

One question in line....

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
File courgette/disassembler_win32_x64.cc (right):

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:466: if (p + 5 <= end_pointer) {
This CL isn't introducing the pattern, but shouldn't each of these if blocks be
if/else blocks?

If we get a hit, we shouldn't keep looking for another match until we've
processed the first hit. Right?

[rickyz (no longer on Chrome), 2015-03-10 05:13:50]

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
File courgette/disassembler_win32_x64.cc (right):

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:469: is_rip_relative = false;
Fine if you prefer it this way, but no need to set it explicitly if it's already
initialized to false.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:473: if (*p == 0x0F  &&  (*(p+1) & 0xF0) ==
0x80) {  // Jcc long form
nit: Could we use p[1] or *(p + 1) consistently everywhere?

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:485: if ((*p & 0xFE) == 0x48 && *(p + 1) ==
0x8D &&
Did you mean to use 0xFB here and below? Otherwise this will miss instructions
that load into r8-r15.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:496: }
Out of curiosity, how do we decide which instructions to consider vs. not here?
(For example, we don't support MOV r/m64,r64 or things that take immediates, or
other instructions like cmp, etc.)

[Will Harris, 2015-03-12 05:33:28]

Thanks for the reviews!  Ricky - supporting r8-r15 gave an extra 1% reduction in
patch size so good spot on that!

PTAL

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
File courgette/disassembler_win32_x64.cc (right):

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:466: if (p + 5 <= end_pointer) {
On 2015/03/10 03:03:48, dgarrett wrote:
> This CL isn't introducing the pattern, but shouldn't each of these if blocks
be
> if/else blocks?
> 
> If we get a hit, we shouldn't keep looking for another match until we've
> processed the first hit. Right?

yes, none of the blocks will ever collide with each other as they read the
instructions and they are all mutually exclusive.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:469: is_rip_relative = false;
On 2015/03/10 05:13:50, rickyz wrote:
> Fine if you prefer it this way, but no need to set it explicitly if it's
already
> initialized to false.

Done.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:473: if (*p == 0x0F  &&  (*(p+1) & 0xF0) ==
0x80) {  // Jcc long form
On 2015/03/10 05:13:50, rickyz wrote:
> nit: Could we use p[1] or *(p + 1) consistently everywhere?

Done.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:485: if ((*p & 0xFE) == 0x48 && *(p + 1) ==
0x8D &&
On 2015/03/10 05:13:50, rickyz wrote:
> Did you mean to use 0xFB here and below? Otherwise this will miss instructions
> that load into r8-r15.

Done.  Good spot.

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:496: }
On 2015/03/10 05:13:50, rickyz wrote:
> Out of curiosity, how do we decide which instructions to consider vs. not
here?
> (For example, we don't support MOV r/m64,r64 or things that take immediates,
or
> other instructions like cmp, etc.)

I agree that there is probably still some scope here for expanding this function
to cope with even more instructions.

[rickyz (no longer on Chrome), 2015-03-12 05:36:25]

lgtm

[dgarrett, 2015-03-12 16:40:54]

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
File courgette/disassembler_win32_x64.cc (right):

https://codereview.chromium.org/878043002/diff/20001/courgette/disassembler_w...
courgette/disassembler_win32_x64.cc:466: if (p + 5 <= end_pointer) {
On 2015/03/12 05:33:28, Will Harris wrote:
> On 2015/03/10 03:03:48, dgarrett wrote:
> > This CL isn't introducing the pattern, but shouldn't each of these if blocks
> be
> > if/else blocks?
> > 
> > If we get a hit, we shouldn't keep looking for another match until we've
> > processed the first hit. Right?
> 
> yes, none of the blocks will ever collide with each other as they read the
> instructions and they are all mutually exclusive.

For some reason, I read the "rel32 = p + 2" type lines as advancing p, nvm.

[dgarrett, 2015-03-12 16:55:56]

lgtm

[Will Harris, 2015-03-12 17:00:47]

Holding off committing this CL until teamspeak sign the Chromium CLA.

[Will Harris, 2015-03-18 23:32:02]

The CQ bit was checked by wfh@chromium.org

[commit-bot: I haz the power, 2015-03-18 23:32:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/878043002/60001

[commit-bot: I haz the power, 2015-03-18 23:36:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-18 23:36:14]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Will Harris, 2015-03-20 05:00:29]

The CQ bit was checked by wfh@chromium.org

[Will Harris, 2015-03-20 05:00:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgarrett@chromium.org, rickyz@chromium.org
Link to the patchset: https://codereview.chromium.org/878043002/#ps80001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-20 05:00:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/878043002/80001

[commit-bot: I haz the power, 2015-03-20 05:06:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 05:06:13]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Will Harris, 2015-03-20 05:16:48]

The CQ bit was checked by wfh@chromium.org

[Will Harris, 2015-03-20 05:16:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgarrett@chromium.org, rickyz@chromium.org
Link to the patchset: https://codereview.chromium.org/878043002/#ps100001
(title: "fix clang build")

[commit-bot: I haz the power, 2015-03-20 05:16:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/878043002/100001

[commit-bot: I haz the power, 2015-03-20 06:23:24]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-20 06:24:41]

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