[minidump] Fix unaligned pointer in thread name list

https://crrev.com/c/3671775/ introduced a warning (and thus, a
compilation failure) on 32-bit ARM when taking the address of the RVA64
field MINIDUMP_THREAD_NAME::RvaOfThreadName:

minidump/minidump_thread_name_list_writer.cc:57:23: error: taking address of packed member 'RvaOfThreadName' of class or structure 'MINIDUMP_THREAD_NAME' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
  name_->RegisterRVA(&thread_name_.RvaOfThreadName);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Indeed, MINIDUMP_THREAD_NAME's RvaOfThreadName field is not aligned,
so the technique used in MinidumpWritable::Register*() of passing in a
rawptr to an arbitrary struct field which is later dereferenced cannot
be used for this field.

This CL replaces the use of MinidumpWritable::Register*() with
overriding MinidumpThreadNameWriter::WillWriteAtOffsetImpl() to
directly calculate and assign thread_name_.RvaOfThreadName.

Change-Id: I71e751a5b5e896b5e7277879bdbdff6e9eefe023
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3693846
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Ben Hamilton <benhamilton@google.com>
Reviewed-by: Ben Hamilton <benhamilton@google.com>
This commit is contained in:
Ben Hamilton 2022-06-08 12:37:49 -06:00 committed by Crashpad LUCI CQ
parent 0662aeb83e
commit 339b125241
3 changed files with 19 additions and 17 deletions

View File

@ -43,18 +43,6 @@ void MinidumpThreadNameWriter::SetThreadName(const std::string& name) {
name_->SetUTF8(name);
}
bool MinidumpThreadNameWriter::Freeze() {
DCHECK_EQ(state(), kStateMutable);
if (!MinidumpWritable::Freeze()) {
return false;
}
name_->RegisterRVA(&thread_name_.RvaOfThreadName);
return true;
}
size_t MinidumpThreadNameWriter::SizeOfObject() {
DCHECK_GE(state(), kStateFrozen);
@ -75,6 +63,24 @@ std::vector<internal::MinidumpWritable*> MinidumpThreadNameWriter::Children() {
return children;
}
bool MinidumpThreadNameWriter::WillWriteAtOffsetImpl(FileOffset offset) {
DCHECK_EQ(state(), kStateFrozen);
// This cannot use RegisterRVA(&thread_name_.RvaOfThreadName), since
// &MINIDUMP_THREAD_NAME_LIST::RvaOfThreadName is not aligned on a pointer
// boundary, so it causes failures on 32-bit ARM.
//
// Instead, manually update the RVA64 to the current file offset since the
// child thread_name_ will write its contents at that offset.
decltype(thread_name_.RvaOfThreadName) local_rva_of_thread_name;
if (!AssignIfInRange(&local_rva_of_thread_name, offset)) {
LOG(ERROR) << "offset " << offset << " out of range";
return false;
}
thread_name_.RvaOfThreadName = local_rva_of_thread_name;
return MinidumpWritable::WillWriteAtOffsetImpl(offset);
}
bool MinidumpThreadNameWriter::WriteObject(FileWriterInterface* file_writer) {
DCHECK_EQ(state(), kStateWritable);

View File

@ -61,9 +61,9 @@ class MinidumpThreadNameWriter final : public internal::MinidumpWritable {
private:
// MinidumpWritable:
bool Freeze() override;
size_t SizeOfObject() override;
std::vector<MinidumpWritable*> Children() override;
bool WillWriteAtOffsetImpl(FileOffset offset) override;
bool WriteObject(FileWriterInterface* file_writer) override;
MINIDUMP_THREAD_NAME thread_name_;

View File

@ -70,8 +70,6 @@ class MinidumpWritable {
// This is public instead of protected because objects of derived classes need
// to be able to register their own pointers with distinct objects.
void RegisterRVA(RVA* rva);
//! \brief 64-bit specialization of RegisterRVA.
void RegisterRVA(RVA64* rva);
//! \brief Registers a location descriptor as one that should point to the
@ -91,8 +89,6 @@ class MinidumpWritable {
// to be able to register their own pointers with distinct objects.
void RegisterLocationDescriptor(
MINIDUMP_LOCATION_DESCRIPTOR* location_descriptor);
//! \brief 64-bit specialization of RegisterLocationDescriptor.
void RegisterLocationDescriptor(
MINIDUMP_LOCATION_DESCRIPTOR64* location_descriptor64);