Fix more MSVC warnings, courgette/ edition.

courgette/disassembler_win32_x64.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "courgette/disassembler_win32_x64.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 555 matching lines @@
         p += relocs_size;
         continue;
       }
     }
 
     while (abs32_pos != abs32_locations_.end() && *abs32_pos < current_rva)
       ++abs32_pos;
 
     if (abs32_pos != abs32_locations_.end() && *abs32_pos == current_rva) {
       uint32 target_address = Read32LittleEndian(p);
-      RVA target_rva = target_address - image_base();
+      // !!! Is this cast safe?

[Peter Kasting, 2014/09/30 00:08:01]

This question must be answered before this change can be committed.

It worries me that we're just blindly downcasting a uint64 here, especially when
the comments on the declaration of image_base() suggest that it really can
require > 32 bits.  The assumption that this fits in 32 bits seems pretty baked
in to the code here, so it's not clear to me what to do.

[dgarrett, 2014/10/01 20:49:37]

There were repeated assurances that it is safe for Win64, but I can see how it
might break. I think that adding a comment describing the concerns is very
reasonable. An error check to blow up if the assumption is wrong it also
reasonable.

However, allowing the RVA sizes to vary based on platform would be a much larger
change than than was done, and increasing them to 64 bits on all platforms would
bloat patch sizes.

I'd have to re-read code for a while to decide if getting this wrong will lead
to invalid patches or just bloated patches.

[Peter Kasting, 2014/10/01 22:12:05]

OK.  I implemented this by way of a checked_cast<>, which will CHECK if the
assumption is violated.  Let me know if you meant for failure in this case to do
something less severe, like just returning false.

+      RVA target_rva = target_address - static_cast<uint32>(image_base());
       // TODO(sra): target could be Label+offset.  It is not clear how to guess
       // which it might be.  We assume offset==0.
       if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
         return false;
       p += 4;
       continue;
     }
 
     while (rel32_pos != rel32_locations_.end() && *rel32_pos < current_rva)
       ++rel32_pos;
@@ skipping 136 matching lines @@
     directory->size_ = static_cast<uint32>(size);
     return true;
   } else {
     directory->address_ = 0;
     directory->size_ = 0;
     return true;
   }
 }
 
 }  // namespace courgette