From f83530bf9a0bae0eb4c01192056576284646757f Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Thu, 10 Nov 2016 14:39:51 -0500 Subject: [PATCH] =?UTF-8?q?GCC=20fix:=20Don=E2=80=99t=20use=20arraysize()?= =?UTF-8?q?=20on=20packed=20structs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While compiling, for example, minidump_exception_writer.cc: In file included from ../../minidump/minidump_exception_writer.h:26:0, from ../../minidump/minidump_exception_writer.cc:15: ../../minidump/minidump_exception_writer.cc: In member function ‘void crashpad::MinidumpExceptionWriter::SetExceptionInformation(const std::vector&)’: ../../minidump/minidump_exception_writer.cc:67:44: error: cannot bind packed field ‘((crashpad::MinidumpExceptionWriter*)this)->crashpad::MinidumpExceptionWriter::exception_.MINIDUMP_EXCEPTION_STREAM::ExceptionRecord.MINIDUMP_EXCEPTION::ExceptionInformation’ to ‘long unsigned int (&)[15]’ arraysize(exception_.ExceptionRecord.ExceptionInformation); ~~~~~~~~~~~~~~~~~~~~~~~~~~~^ ../../third_party/mini_chromium/mini_chromium/base/macros.h:41:50: note: in definition of macro ‘arraysize’ #define arraysize(array) (sizeof(ArraySizeHelper(array))) Tested with: - GCC 4.9 from NDK r13 targeting arm with SDK 16 - GCC 4.9 from NDK r13 targeting arm64 with SDK 21 - GCC 6.2 targeting x86_64 BUG=crashpad:30 Change-Id: I63963b277a309b4715148215f51902c33ba13b5a Reviewed-on: https://chromium-review.googlesource.com/409694 Reviewed-by: Scott Graham --- minidump/minidump_exception_writer.cc | 3 +- minidump/minidump_misc_info_writer.cc | 11 ++-- minidump/minidump_system_info_writer.cc | 8 ++- util/misc/arraysize_unsafe.h | 27 +++++++++ util/misc/arraysize_unsafe_test.cc | 73 +++++++++++++++++++++++++ util/util.gyp | 1 + util/util_test.gyp | 1 + 7 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 util/misc/arraysize_unsafe.h create mode 100644 util/misc/arraysize_unsafe_test.cc diff --git a/minidump/minidump_exception_writer.cc b/minidump/minidump_exception_writer.cc index 5ec8d374..d2466a6b 100644 --- a/minidump/minidump_exception_writer.cc +++ b/minidump/minidump_exception_writer.cc @@ -21,6 +21,7 @@ #include "minidump/minidump_context_writer.h" #include "snapshot/exception_snapshot.h" #include "util/file/file_writer.h" +#include "util/misc/arraysize_unsafe.h" namespace crashpad { @@ -64,7 +65,7 @@ void MinidumpExceptionWriter::SetExceptionInformation( const size_t parameters = exception_information.size(); const size_t kMaxParameters = - arraysize(exception_.ExceptionRecord.ExceptionInformation); + ARRAYSIZE_UNSAFE(exception_.ExceptionRecord.ExceptionInformation); CHECK_LE(parameters, kMaxParameters); exception_.ExceptionRecord.NumberParameters = diff --git a/minidump/minidump_misc_info_writer.cc b/minidump/minidump_misc_info_writer.cc index 5f88c50c..ff4fdda8 100644 --- a/minidump/minidump_misc_info_writer.cc +++ b/minidump/minidump_misc_info_writer.cc @@ -26,6 +26,7 @@ #include "snapshot/process_snapshot.h" #include "snapshot/system_snapshot.h" #include "util/file/file_writer.h" +#include "util/misc/arraysize_unsafe.h" #include "util/numeric/in_range_cast.h" #include "util/numeric/safe_assignment.h" @@ -295,7 +296,7 @@ void MinidumpMiscInfoWriter::SetTimeZone(uint32_t time_zone_id, internal::MinidumpWriterUtil::AssignUTF8ToUTF16( misc_info_.TimeZone.StandardName, - arraysize(misc_info_.TimeZone.StandardName), + ARRAYSIZE_UNSAFE(misc_info_.TimeZone.StandardName), standard_name); misc_info_.TimeZone.StandardDate = standard_date; @@ -303,7 +304,7 @@ void MinidumpMiscInfoWriter::SetTimeZone(uint32_t time_zone_id, internal::MinidumpWriterUtil::AssignUTF8ToUTF16( misc_info_.TimeZone.DaylightName, - arraysize(misc_info_.TimeZone.DaylightName), + ARRAYSIZE_UNSAFE(misc_info_.TimeZone.DaylightName), daylight_name); misc_info_.TimeZone.DaylightDate = daylight_date; @@ -320,10 +321,12 @@ void MinidumpMiscInfoWriter::SetBuildString( misc_info_.Flags1 |= MINIDUMP_MISC4_BUILDSTRING; internal::MinidumpWriterUtil::AssignUTF8ToUTF16( - misc_info_.BuildString, arraysize(misc_info_.BuildString), build_string); + misc_info_.BuildString, + ARRAYSIZE_UNSAFE(misc_info_.BuildString), + build_string); internal::MinidumpWriterUtil::AssignUTF8ToUTF16( misc_info_.DbgBldStr, - arraysize(misc_info_.DbgBldStr), + ARRAYSIZE_UNSAFE(misc_info_.DbgBldStr), debug_build_string); } diff --git a/minidump/minidump_system_info_writer.cc b/minidump/minidump_system_info_writer.cc index 5b7ca8f4..9c665f4f 100644 --- a/minidump/minidump_system_info_writer.cc +++ b/minidump/minidump_system_info_writer.cc @@ -20,6 +20,7 @@ #include "minidump/minidump_string_writer.h" #include "snapshot/system_snapshot.h" #include "util/file/file_writer.h" +#include "util/misc/arraysize_unsafe.h" #include "util/misc/implicit_cast.h" namespace crashpad { @@ -196,7 +197,7 @@ void MinidumpSystemInfoWriter::SetCPUX86Vendor(uint32_t ebx, system_info_.ProcessorArchitecture == kMinidumpCPUArchitectureX86Win64); - static_assert(arraysize(system_info_.Cpu.X86CpuInfo.VendorId) == 3, + static_assert(ARRAYSIZE_UNSAFE(system_info_.Cpu.X86CpuInfo.VendorId) == 3, "VendorId must have 3 elements"); system_info_.Cpu.X86CpuInfo.VendorId[0] = ebx; @@ -254,8 +255,9 @@ void MinidumpSystemInfoWriter::SetCPUOtherFeatures(uint64_t features_0, system_info_.ProcessorArchitecture != kMinidumpCPUArchitectureX86Win64); - static_assert(arraysize(system_info_.Cpu.OtherCpuInfo.ProcessorFeatures) == 2, - "ProcessorFeatures must have 2 elements"); + static_assert( + ARRAYSIZE_UNSAFE(system_info_.Cpu.OtherCpuInfo.ProcessorFeatures) == 2, + "ProcessorFeatures must have 2 elements"); system_info_.Cpu.OtherCpuInfo.ProcessorFeatures[0] = features_0; system_info_.Cpu.OtherCpuInfo.ProcessorFeatures[1] = features_1; diff --git a/util/misc/arraysize_unsafe.h b/util/misc/arraysize_unsafe.h new file mode 100644 index 00000000..e53c70b8 --- /dev/null +++ b/util/misc/arraysize_unsafe.h @@ -0,0 +1,27 @@ +// Copyright 2016 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_ARRAYSIZE_UNSAFE_H_ +#define CRASHPAD_UTIL_MISC_ARRAYSIZE_UNSAFE_H_ + +//! \file + +//! \brief Not the safest way of computing an array’s size… +//! +//! `#%include "base/macros.h"` and use its `arraysize()` instead. This macro +//! should only be used in rare situations where `arraysize()` does not +//! function. +#define ARRAYSIZE_UNSAFE(array) (sizeof(array) / sizeof(array[0])) + +#endif // CRASHPAD_UTIL_MISC_ARRAYSIZE_UNSAFE_H_ diff --git a/util/misc/arraysize_unsafe_test.cc b/util/misc/arraysize_unsafe_test.cc new file mode 100644 index 00000000..1c638e02 --- /dev/null +++ b/util/misc/arraysize_unsafe_test.cc @@ -0,0 +1,73 @@ +// Copyright 2016 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/arraysize_unsafe.h" + +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(ArraySizeUnsafe, ArraySizeUnsafe) { + char c0[0]; + static_assert(ARRAYSIZE_UNSAFE(c0) == 0, "c0"); + + char c1[1]; + static_assert(ARRAYSIZE_UNSAFE(c1) == 1, "c1"); + + char c2[2]; + static_assert(ARRAYSIZE_UNSAFE(c2) == 2, "c2"); + + char c4[4]; + static_assert(ARRAYSIZE_UNSAFE(c4) == 4, "c4"); + + int i0[0]; + static_assert(ARRAYSIZE_UNSAFE(i0) == 0, "i0"); + + int i1[1]; + static_assert(ARRAYSIZE_UNSAFE(i1) == 1, "i1"); + + int i2[2]; + static_assert(ARRAYSIZE_UNSAFE(i2) == 2, "i2"); + + int i4[4]; + static_assert(ARRAYSIZE_UNSAFE(i4) == 4, "i4"); + + long l8[8]; + static_assert(ARRAYSIZE_UNSAFE(l8) == 8, "l8"); + + int l9[9]; + static_assert(ARRAYSIZE_UNSAFE(l9) == 9, "l9"); + + struct S { + char c; + int i; + long l; + bool b; + }; + + S s0[0]; + static_assert(ARRAYSIZE_UNSAFE(s0) == 0, "s0"); + + S s1[1]; + static_assert(ARRAYSIZE_UNSAFE(s1) == 1, "s1"); + + S s10[10]; + static_assert(ARRAYSIZE_UNSAFE(s10) == 10, "s10"); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index 8269bd42..5768788a 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -84,6 +84,7 @@ 'mach/task_for_pid.h', 'mach/task_memory.cc', 'mach/task_memory.h', + 'misc/arraysize_unsafe.h', 'misc/clock.h', 'misc/clock_mac.cc', 'misc/clock_posix.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index 9efe162a..b5989de9 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -55,6 +55,7 @@ 'mach/scoped_task_suspend_test.cc', 'mach/symbolic_constants_mach_test.cc', 'mach/task_memory_test.cc', + 'misc/arraysize_unsafe_test.cc', 'misc/clock_test.cc', 'misc/initialization_state_dcheck_test.cc', 'misc/initialization_state_test.cc',