Introduce FromPointerCast<>(), with defined sign/zero-extension behavior

Some of the new Linux/Android tests were failing in 32-bit code where
pointers were being casted via reinterpret_cast<>() to LinuxVMAddress,
an unsigned 64-bit type. The behavior of such casts is
implementation-defined, and in this case, sign-extension was being used
to convert the 32-bit pointers to 64 bits, resulting in very large
(unsigned) LinuxVMAddress values that could not possibly refer to proper
addresses in a 32-bit process’ address space.

The offending reinterpret_cast<>() conversions have been replaced with
the new FromPointerCast<>(), which is careful to do sign-extension when
converting to a signed type, and zero-extension when converting to an
unsigned type like LinuxVMAddress.

Bug: crashpad:30
Test: crashpad_util_test FromPointerCast*:MemoryMap.*:ProcessMemory.*
Change-Id: I6f1408dc63369a8740ecd6015d657e4407a7c271
Reviewed-on: https://chromium-review.googlesource.com/488264
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Mark Mentovai 2017-04-27 15:08:17 -04:00
parent ed8e637817
commit 984749479f
7 changed files with 303 additions and 11 deletions

View File

@ -21,6 +21,7 @@
#include <unistd.h>
#include "base/files/file_path.h"
#include "base/strings/stringprintf.h"
#include "gtest/gtest.h"
#include "test/errors.h"
#include "test/file.h"
@ -28,6 +29,7 @@
#include "test/scoped_temp_dir.h"
#include "util/file/file_io.h"
#include "util/misc/clock.h"
#include "util/misc/from_pointer_cast.h"
#include "util/posix/scoped_mmap.h"
namespace crashpad {
@ -45,7 +47,7 @@ TEST(MemoryMap, SelfBasic) {
MemoryMap map;
ASSERT_TRUE(map.Initialize(getpid()));
auto stack_address = reinterpret_cast<LinuxVMAddress>(&map);
auto stack_address = FromPointerCast<LinuxVMAddress>(&map);
const MemoryMap::Mapping* mapping = map.FindMapping(stack_address);
ASSERT_TRUE(mapping);
EXPECT_GE(stack_address, mapping->range.Base());
@ -53,7 +55,7 @@ TEST(MemoryMap, SelfBasic) {
EXPECT_TRUE(mapping->readable);
EXPECT_TRUE(mapping->writable);
auto code_address = reinterpret_cast<LinuxVMAddress>(getpid);
auto code_address = FromPointerCast<LinuxVMAddress>(getpid);
mapping = map.FindMapping(code_address);
ASSERT_TRUE(mapping);
EXPECT_GE(code_address, mapping->range.Base());
@ -146,10 +148,10 @@ class MapChildTest : public Multiprocess {
}
void MultiprocessChild() override {
auto code_address = reinterpret_cast<LinuxVMAddress>(getpid);
auto code_address = FromPointerCast<LinuxVMAddress>(getpid);
CheckedWriteFile(WritePipeHandle(), &code_address, sizeof(code_address));
auto stack_address = reinterpret_cast<LinuxVMAddress>(&code_address);
auto stack_address = FromPointerCast<LinuxVMAddress>(&code_address);
CheckedWriteFile(WritePipeHandle(), &stack_address, sizeof(stack_address));
ScopedMmap mapping;
@ -227,6 +229,8 @@ void ExpectMappings(const MemoryMap& map,
size_t num_mappings,
size_t mapping_size) {
for (size_t index = 0; index < num_mappings; ++index) {
SCOPED_TRACE(base::StringPrintf("index %zu", index));
auto mapping_address = region_addr + index * mapping_size;
const MemoryMap::Mapping* mapping = map.FindMapping(mapping_address);
ASSERT_TRUE(mapping);
@ -269,10 +273,12 @@ class MapRunningChildTest : public Multiprocess {
LinuxVMAddress region_addr;
CheckedReadFileExactly(ReadPipeHandle(), &region_addr, sizeof(region_addr));
for (int iter = 0; iter < 8; ++iter) {
SCOPED_TRACE(base::StringPrintf("iter %d", iter));
// Let the child get back to its work
SleepNanoseconds(1000);
for (int iter = 0; iter < 8; ++iter) {
MemoryMap map;
ASSERT_TRUE(map.Initialize(ChildPID()));

View File

@ -24,6 +24,7 @@
#include "test/errors.h"
#include "test/multiprocess.h"
#include "util/file/file_io.h"
#include "util/misc/from_pointer_cast.h"
#include "util/posix/scoped_mmap.h"
namespace crashpad {
@ -66,7 +67,7 @@ class ReadTest : public TargetProcessTest {
ProcessMemory memory;
ASSERT_TRUE(memory.Initialize(pid));
LinuxVMAddress address = reinterpret_cast<LinuxVMAddress>(region_.get());
LinuxVMAddress address = FromPointerCast<LinuxVMAddress>(region_.get());
std::unique_ptr<char[]> result(new char[region_size_]);
// Ensure that the entire region can be read.
@ -123,7 +124,7 @@ TEST(ProcessMemory, ReadForked) {
bool ReadCString(const ProcessMemory& memory,
const char* pointer,
std::string* result) {
return memory.ReadCString(reinterpret_cast<LinuxVMAddress>(pointer), result);
return memory.ReadCString(FromPointerCast<LinuxVMAddress>(pointer), result);
}
bool ReadCStringSizeLimited(const ProcessMemory& memory,
@ -131,7 +132,7 @@ bool ReadCStringSizeLimited(const ProcessMemory& memory,
size_t size,
std::string* result) {
return memory.ReadCStringSizeLimited(
reinterpret_cast<LinuxVMAddress>(pointer), size, result);
FromPointerCast<LinuxVMAddress>(pointer), size, result);
}
const char kConstCharEmpty[] = "";

View File

@ -0,0 +1,91 @@
// Copyright 2017 The Crashpad Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef CRASHPAD_UTIL_MISC_FROM_POINTER_CAST_H_
#define CRASHPAD_UTIL_MISC_FROM_POINTER_CAST_H_
#include <stdint.h>
#include <cstddef>
#include <type_traits>
namespace crashpad {
#if DOXYGEN
//! \brief Casts from a pointer type to an integer.
//!
//! Compared to `reinterpret_cast<>()`, FromPointerCast<>() defines whether a
//! pointer type is sign-extended or zero-extended. Casts to signed integral
//! types are sign-extended. Casts to unsigned integral types are zero-extended.
template <typename To, typename From>
FromPointerCast(const From from) {
return reinterpret_cast<To>(from);
}
#else // DOXYGEN
// Cast std::nullptr_t to any pointer type.
//
// In C++14, the nullptr_t check could use std::is_null_pointer<From>::type
// instead of the is_same<remove_cv<From>::type, nullptr_t>::type construct.
template <typename To, typename From>
typename std::enable_if<
std::is_same<typename std::remove_cv<From>::type, std::nullptr_t>::value &&
std::is_pointer<To>::value,
To>::type
FromPointerCast(const From& from) {
return static_cast<To>(from);
}
// Cast std::nullptr_t to any integral type.
//
// In C++14, the nullptr_t check could use std::is_null_pointer<From>::type
// instead of the is_same<remove_cv<From>::type, nullptr_t>::type construct.
template <typename To, typename From>
typename std::enable_if<
std::is_same<typename std::remove_cv<From>::type, std::nullptr_t>::value &&
std::is_integral<To>::value,
To>::type
FromPointerCast(const From& from) {
return reinterpret_cast<To>(from);
}
// Cast a pointer to any other pointer type.
template <typename To, typename From>
typename std::enable_if<std::is_pointer<From>::value &&
std::is_pointer<To>::value,
To>::type
FromPointerCast(const From from) {
return reinterpret_cast<To>(from);
}
// Cast a pointer to an integral type. Sign-extend when casting to a signed
// type, zero-extend when casting to an unsigned type.
template <typename To, typename From>
typename std::enable_if<std::is_pointer<From>::value &&
std::is_integral<To>::value,
To>::type
FromPointerCast(const From from) {
return static_cast<To>(
reinterpret_cast<typename std::conditional<std::is_signed<To>::value,
intptr_t,
uintptr_t>::type>(from));
}
#endif // DOXYGEN
} // namespace crashpad
#endif // CRASHPAD_UTIL_MISC_FROM_POINTER_CAST_H_

View File

@ -0,0 +1,190 @@
// Copyright 2017 The Crashpad Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include "util/misc/from_pointer_cast.h"
#include <stdlib.h>
#include <sys/types.h>
#include <limits>
#include "build/build_config.h"
#include "gtest/gtest.h"
namespace crashpad {
namespace test {
namespace {
struct SomeType {};
template <typename T>
class FromPointerCastTest : public testing::Test {};
using FromPointerCastTestTypes = testing::Types<void*,
const void*,
volatile void*,
const volatile void*,
SomeType*,
const SomeType*,
volatile SomeType*,
const volatile SomeType*>;
TYPED_TEST_CASE(FromPointerCastTest, FromPointerCastTestTypes);
TYPED_TEST(FromPointerCastTest, ToSigned) {
EXPECT_EQ(FromPointerCast<int64_t>(nullptr), 0);
EXPECT_EQ(FromPointerCast<int64_t>(reinterpret_cast<TypeParam>(1)), 1);
EXPECT_EQ(FromPointerCast<int64_t>(reinterpret_cast<TypeParam>(-1)), -1);
EXPECT_EQ(FromPointerCast<int64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<uintptr_t>::max())),
static_cast<intptr_t>(std::numeric_limits<uintptr_t>::max()));
EXPECT_EQ(FromPointerCast<int64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<intptr_t>::min())),
std::numeric_limits<intptr_t>::min());
EXPECT_EQ(FromPointerCast<int64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<intptr_t>::max())),
std::numeric_limits<intptr_t>::max());
}
TYPED_TEST(FromPointerCastTest, ToUnsigned) {
EXPECT_EQ(FromPointerCast<uint64_t>(nullptr), 0u);
EXPECT_EQ(FromPointerCast<uint64_t>(reinterpret_cast<TypeParam>(1)), 1u);
EXPECT_EQ(FromPointerCast<uint64_t>(reinterpret_cast<TypeParam>(-1)),
static_cast<uintptr_t>(-1));
EXPECT_EQ(FromPointerCast<uint64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<uintptr_t>::max())),
std::numeric_limits<uintptr_t>::max());
EXPECT_EQ(FromPointerCast<uint64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<intptr_t>::min())),
static_cast<uintptr_t>(std::numeric_limits<intptr_t>::min()));
EXPECT_EQ(FromPointerCast<uint64_t>(reinterpret_cast<TypeParam>(
std::numeric_limits<intptr_t>::max())),
static_cast<uintptr_t>(std::numeric_limits<intptr_t>::max()));
}
// MatchCV<YourType, CVQualifiedType>::Type adapts YourType to match the
// const/void qualification of CVQualifiedType.
template <typename Base, typename MatchTo>
struct MatchCV {
private:
using NonCVBase = typename std::remove_cv<Base>::type;
public:
using Type = typename std::conditional<
std::is_const<MatchTo>::value,
typename std::conditional<std::is_volatile<MatchTo>::value,
const volatile NonCVBase,
const NonCVBase>::type,
typename std::conditional<std::is_volatile<MatchTo>::value,
volatile NonCVBase,
NonCVBase>::type>::type;
};
#if defined(COMPILER_MSVC) && _MSC_VER < 1910
// gtest under MSVS 2015 (MSC 19.0) doesnt handle EXPECT_EQ(a, b) when a or b
// is a pointer to a volatile type, because it cant figure out how to print
// them.
template <typename T>
typename std::remove_volatile<typename std::remove_pointer<T>::type>::type*
MaybeRemoveVolatile(const T& value) {
return const_cast<typename std::remove_volatile<
typename std::remove_pointer<T>::type>::type*>(value);
}
#else // COMPILER_MSVC && _MSC_VER < 1910
// This isnt a problem in MSVS 2017 (MSC 19.1) or with other compilers.
template <typename T>
T MaybeRemoveVolatile(const T& value) {
return value;
}
#endif // COMPILER_MSVC && _MSC_VER < 1910
TYPED_TEST(FromPointerCastTest, ToPointer) {
using CVSomeType =
typename MatchCV<SomeType,
typename std::remove_pointer<TypeParam>::type>::Type;
EXPECT_EQ(MaybeRemoveVolatile(FromPointerCast<CVSomeType*>(nullptr)),
MaybeRemoveVolatile(static_cast<CVSomeType*>(nullptr)));
EXPECT_EQ(MaybeRemoveVolatile(
FromPointerCast<CVSomeType*>(reinterpret_cast<TypeParam>(1))),
MaybeRemoveVolatile(reinterpret_cast<CVSomeType*>(1)));
EXPECT_EQ(MaybeRemoveVolatile(
FromPointerCast<CVSomeType*>(reinterpret_cast<TypeParam>(-1))),
MaybeRemoveVolatile(reinterpret_cast<CVSomeType*>(-1)));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<CVSomeType*>(
reinterpret_cast<TypeParam>(std::numeric_limits<uintptr_t>::max()))),
MaybeRemoveVolatile(reinterpret_cast<CVSomeType*>(
std::numeric_limits<uintptr_t>::max())));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<CVSomeType*>(
reinterpret_cast<TypeParam>(std::numeric_limits<intptr_t>::min()))),
MaybeRemoveVolatile(
reinterpret_cast<CVSomeType*>(std::numeric_limits<intptr_t>::min())));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<CVSomeType*>(
reinterpret_cast<TypeParam>(std::numeric_limits<intptr_t>::max()))),
MaybeRemoveVolatile(
reinterpret_cast<CVSomeType*>(std::numeric_limits<intptr_t>::max())));
}
TEST(FromPointerCast, FromFunctionPointer) {
// These casts should work with or without the & in &malloc.
EXPECT_NE(FromPointerCast<int64_t>(malloc), 0);
EXPECT_NE(FromPointerCast<int64_t>(&malloc), 0);
EXPECT_NE(FromPointerCast<uint64_t>(malloc), 0u);
EXPECT_NE(FromPointerCast<uint64_t>(&malloc), 0u);
EXPECT_EQ(FromPointerCast<void*>(malloc), reinterpret_cast<void*>(malloc));
EXPECT_EQ(FromPointerCast<void*>(&malloc), reinterpret_cast<void*>(malloc));
EXPECT_EQ(FromPointerCast<const void*>(malloc),
reinterpret_cast<const void*>(malloc));
EXPECT_EQ(FromPointerCast<const void*>(&malloc),
reinterpret_cast<const void*>(malloc));
EXPECT_EQ(MaybeRemoveVolatile(FromPointerCast<volatile void*>(malloc)),
MaybeRemoveVolatile(reinterpret_cast<volatile void*>(malloc)));
EXPECT_EQ(MaybeRemoveVolatile(FromPointerCast<volatile void*>(&malloc)),
MaybeRemoveVolatile(reinterpret_cast<volatile void*>(malloc)));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<const volatile void*>(malloc)),
MaybeRemoveVolatile(reinterpret_cast<const volatile void*>(malloc)));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<const volatile void*>(&malloc)),
MaybeRemoveVolatile(reinterpret_cast<const volatile void*>(malloc)));
EXPECT_EQ(FromPointerCast<SomeType*>(malloc),
reinterpret_cast<SomeType*>(malloc));
EXPECT_EQ(FromPointerCast<SomeType*>(&malloc),
reinterpret_cast<SomeType*>(malloc));
EXPECT_EQ(FromPointerCast<const SomeType*>(malloc),
reinterpret_cast<const SomeType*>(malloc));
EXPECT_EQ(FromPointerCast<const SomeType*>(&malloc),
reinterpret_cast<const SomeType*>(malloc));
EXPECT_EQ(MaybeRemoveVolatile(FromPointerCast<volatile SomeType*>(malloc)),
MaybeRemoveVolatile(reinterpret_cast<volatile SomeType*>(malloc)));
EXPECT_EQ(MaybeRemoveVolatile(FromPointerCast<volatile SomeType*>(&malloc)),
MaybeRemoveVolatile(reinterpret_cast<volatile SomeType*>(malloc)));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<const volatile SomeType*>(malloc)),
MaybeRemoveVolatile(reinterpret_cast<const volatile SomeType*>(malloc)));
EXPECT_EQ(
MaybeRemoveVolatile(FromPointerCast<const volatile SomeType*>(&malloc)),
MaybeRemoveVolatile(reinterpret_cast<const volatile SomeType*>(malloc)));
}
} // namespace
} // namespace test
} // namespace crashpad

View File

@ -20,6 +20,8 @@
#include <sys/mman.h>
#include <sys/types.h>
#include "util/misc/from_pointer_cast.h"
namespace crashpad {
//! \brief Maintains a memory-mapped region created by `mmap()`.
@ -85,7 +87,7 @@ class ScopedMmap {
//! a type of the callers choosing.
template <typename T>
T addr_as() const {
return reinterpret_cast<T>(addr_);
return FromPointerCast<T>(addr_);
}
//! \brief Returns the size of the memory-mapped region.

View File

@ -101,6 +101,7 @@
'misc/clock_mac.cc',
'misc/clock_posix.cc',
'misc/clock_win.cc',
'misc/from_pointer_cast.h',
'misc/implicit_cast.h',
'misc/initialization_state.h',
'misc/initialization_state_dcheck.cc',

View File

@ -63,6 +63,7 @@
'mach/task_memory_test.cc',
'misc/arraysize_unsafe_test.cc',
'misc/clock_test.cc',
'misc/from_pointer_cast_test.cc',
'misc/initialization_state_dcheck_test.cc',
'misc/initialization_state_test.cc',
'misc/paths_test.cc',