win: Use signed int as the integer representation of HANDLEs

HandleToInt() and IntToHandle() use int, a signed type, for the 32-bit
integer representation of HANDLE values. For opaque values, an unsigned
type would normally be used, but in this case, signed was chosen for
sign extension to work correctly. INVALID_HANDLE_VALUE is defined as
((HANDLE)(LONG_PTR)-1), and this needs to round-trip through the chosen
integer representation back to the same HANDLE value. Sign extension is
also recommended by
https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203.

As suggested in
https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1422023010 .

test/win/win_child_process.cc

@@ -142,9 +142,13 @@ WinChildProcess::WinChildProcess() {
   // values are passed to the child on the command line.
   std::string left, right;
   CHECK(SplitString(switch_value, '|', &left, &right));
+
+  // left and right were formatted as 0x%x, so they need to be converted as
+  // unsigned ints.
   unsigned int write, read;
   CHECK(StringToNumber(left, &write));
   CHECK(StringToNumber(right, &read));
+
   pipe_write_.reset(IntToHandle(write));
   pipe_read_.reset(IntToHandle(read));
 

util/win/handle.h

@@ -21,17 +21,17 @@ namespace crashpad {
 
 //! \brief Converts a `HANDLE` to an `int`.
 //!
-//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
+//! `HANDLE` is a `typedef` for `void *`, but kernel `HANDLE` values aren’t
-//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
+//! pointers to anything. Only 32 bits of kernel `HANDLE`s are significant, even
-//! even in 64-bit processes on 64-bit operating systems. See <a
+//! in 64-bit processes on 64-bit operating systems. See <a
 //! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
 //! Communication Between 32-bit and 64-bit Applications</a>.
 //!
-//! This function safely converts a shareable `HANDLE` to an `int` similarly to
+//! This function safely converts a kernel `HANDLE` to an `int` similarly to a
-//! a cast operation. It checks that the operation can be performed safely, and
+//! cast operation. It checks that the operation can be performed safely, and
 //! aborts execution if it cannot.
 //!
-//! \param[in] handle The shareable `HANDLE` to convert.
+//! \param[in] handle The kernel `HANDLE` to convert.
 //!
 //! \return An equivalent `int`, truncated (if necessary) from \a handle. If
 //!     truncation would have resulted in an `int` that could not be converted
@@ -42,20 +42,20 @@ int HandleToInt(HANDLE handle);
 
 //! \brief Converts an `int` to an `HANDLE`.
 //!
-//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
+//! `HANDLE` is a `typedef` for `void *`, but kernel `HANDLE` values aren’t
-//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
+//! pointers to anything. Only 32 bits of kernel `HANDLE`s are significant, even
-//! even in 64-bit processes on 64-bit operating systems. See <a
+//! in 64-bit processes on 64-bit operating systems. See <a
 //! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
 //! Communication Between 32-bit and 64-bit Applications</a>.
 //!
-//! This function safely convert an `int` to a shareable `HANDLE` similarly to a
+//! This function safely convert an `int` to a kernel `HANDLE` similarly to a
 //! cast operation.
 //!
 //! \param[in] handle_int The `int` to convert. This must have been produced by
 //!     HandleToInt(), possibly in a different process.
 //!
-//! \return An equivalent shareable `HANDLE`, sign-extended (if necessary) from
+//! \return An equivalent kernel `HANDLE`, sign-extended (if necessary) from \a
-//!     \a handle_int.
+//!     handle_int.
 //!
 //! \sa HandleToInt()
 HANDLE IntToHandle(int handle_int);

util/win/process_info.h

@@ -58,7 +58,7 @@ class ProcessInfo {
     std::wstring type_name;
 
     //! \brief The handle's value.
-    uint32_t handle;
+    int handle;
 
     //! \brief The attributes for the handle, e.g. `OBJ_INHERIT`,
     //!     `OBJ_CASE_INSENSITIVE`, etc.

util/win/process_info_test.cc

@@ -576,8 +576,7 @@ TEST(ProcessInfo, Handles) {
   bool found_key_handle = false;
   bool found_mapping_handle = false;
   for (auto handle : info.Handles()) {
-    const int handle_int = implicit_cast<int>(handle.handle);
+    if (handle.handle == HandleToInt(file.get())) {
-    if (handle_int == HandleToInt(file.get())) {
       EXPECT_FALSE(found_file_handle);
       found_file_handle = true;
       EXPECT_EQ(L"File", handle.type_name);
@@ -587,7 +586,7 @@ TEST(ProcessInfo, Handles) {
                 handle.granted_access & STANDARD_RIGHTS_ALL);
       EXPECT_EQ(0, handle.attributes);
     }
-    if (handle_int == HandleToInt(inherited_file.get())) {
+    if (handle.handle == HandleToInt(inherited_file.get())) {
       EXPECT_FALSE(found_inherited_file_handle);
       found_inherited_file_handle = true;
       EXPECT_EQ(L"File", handle.type_name);
@@ -601,7 +600,7 @@ TEST(ProcessInfo, Handles) {
       const int kObjInherit = 0x2;
       EXPECT_EQ(kObjInherit, handle.attributes);
     }
-    if (handle_int == HandleToInt(scoped_key.get())) {
+    if (handle.handle == HandleToInt(scoped_key.get())) {
       EXPECT_FALSE(found_key_handle);
       found_key_handle = true;
       EXPECT_EQ(L"Key", handle.type_name);
@@ -611,7 +610,7 @@ TEST(ProcessInfo, Handles) {
                 handle.granted_access & STANDARD_RIGHTS_ALL);
       EXPECT_EQ(0, handle.attributes);
     }
-    if (handle_int == HandleToInt(mapping.get())) {
+    if (handle.handle == HandleToInt(mapping.get())) {
       EXPECT_FALSE(found_mapping_handle);
       found_mapping_handle = true;
       EXPECT_EQ(L"Section", handle.type_name);

util/win/registration_protocol_win.h

@@ -92,22 +92,22 @@ struct ClientToServerMessage {
   };
 };
 
-//! \brief  A client registration response.
+//! \brief A client registration response.
 struct RegistrationResponse {
   //! \brief An event `HANDLE`, valid in the client process, that should be
-  //!     signaled to request a crash report. 64-bit clients should convert the
+  //!     signaled to request a crash report. Clients should convert the value
-  //!     value to a `HANDLE` using sign-extension.
+  //!     to a `HANDLE` by calling IntToHandle().
-  uint32_t request_crash_dump_event;
+  int request_crash_dump_event;
 
   //! \brief An event `HANDLE`, valid in the client process, that should be
-  //!     signaled to request a non-crashing dump be taken. 64-bit clients
+  //!     signaled to request a non-crashing dump be taken. Clients should
-  //!     should convert the value to `HANDLEEE` using sign-extension.
+  //!     convert the value to a `HANDLE` by calling IntToHandle().
-  uint32_t request_non_crash_dump_event;
+  int request_non_crash_dump_event;
 
   //! \brief An event `HANDLE`, valid in the client process, that will be
-  //!     signaled by the server when the non-crashing dump is complete. 64-bit
+  //!     signaled by the server when the non-crashing dump is complete. Clients
-  //!     clients should convert the value to `HANDLEEE` using sign-extension.
+  //!     should convert the value to a `HANDLE` by calling IntToHandle().
-  uint32_t non_crash_dump_completed_event;
+  int non_crash_dump_completed_event;
 };
 
 //! \brief The response sent back to the client via SendToCrashHandlerServer().