Fixed alignment issue that caused bus errors on platforms like sparc which
require all memory reads to be aligned. Specifically, it turns out that sizeof(RepeatedField<bool>) is 20 on 64-bit sparc with GCC 3.4.6. This is strange, since one of RepeatedField's members is a pointer, which I thought meant that it had to be 64-bit aligned, which means its size should be a multiple of 64 bits. But, 20 is not a multiple of 8. I don't understand why this is the case, but if this is possible, then DynamicMessage's strategy of sorting fields in descending order by size and then tightly packing doesn't work. To fix this, I got rid of the sort step and instead added code that aligns each field's offset appropriately based on the field's size. Also in this revision: Fix an error message that named a flag incorrectly.
This commit is contained in:
parent
de75437573
commit
d0580eafd3
@ -476,7 +476,7 @@ bool CommandLineInterface::ParseArguments(int argc, const char* const argv[]) {
|
|||||||
}
|
}
|
||||||
if (imports_in_descriptor_set_ && descriptor_set_name_.empty()) {
|
if (imports_in_descriptor_set_ && descriptor_set_name_.empty()) {
|
||||||
cerr << "--include_imports only makes sense when combined with "
|
cerr << "--include_imports only makes sense when combined with "
|
||||||
"--descriptor_set_name." << endl;
|
"--descriptor_set_out." << endl;
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
|
@ -131,36 +131,20 @@ int FieldSpaceUsed(const FieldDescriptor* field) {
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
struct DescendingFieldSizeOrder {
|
|
||||||
inline bool operator()(const FieldDescriptor* a,
|
|
||||||
const FieldDescriptor* b) {
|
|
||||||
// All repeated fields come first.
|
|
||||||
if (a->is_repeated()) {
|
|
||||||
if (b->is_repeated()) {
|
|
||||||
// Repeated fields and are not ordered with respect to each other.
|
|
||||||
return false;
|
|
||||||
} else {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
} else if (b->is_repeated()) {
|
|
||||||
return false;
|
|
||||||
} else {
|
|
||||||
// Remaining fields in descending order by size.
|
|
||||||
return FieldSpaceUsed(a) > FieldSpaceUsed(b);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
inline int DivideRoundingUp(int i, int j) {
|
inline int DivideRoundingUp(int i, int j) {
|
||||||
return (i + (j - 1)) / j;
|
return (i + (j - 1)) / j;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const int kSafeAlignment = sizeof(uint64);
|
static const int kSafeAlignment = sizeof(uint64);
|
||||||
|
|
||||||
|
inline int AlignTo(int offset, int alignment) {
|
||||||
|
return DivideRoundingUp(offset, alignment) * alignment;
|
||||||
|
}
|
||||||
|
|
||||||
// Rounds the given byte offset up to the next offset aligned such that any
|
// Rounds the given byte offset up to the next offset aligned such that any
|
||||||
// type may be stored at it.
|
// type may be stored at it.
|
||||||
inline int AlignOffset(int offset) {
|
inline int AlignOffset(int offset) {
|
||||||
return DivideRoundingUp(offset, kSafeAlignment) * kSafeAlignment;
|
return AlignTo(offset, kSafeAlignment);
|
||||||
}
|
}
|
||||||
|
|
||||||
#define bitsizeof(T) (sizeof(T) * 8)
|
#define bitsizeof(T) (sizeof(T) * 8)
|
||||||
@ -466,18 +450,6 @@ const Message* DynamicMessageFactory::GetPrototype(const Descriptor* type) {
|
|||||||
int* offsets = new int[type->field_count()];
|
int* offsets = new int[type->field_count()];
|
||||||
type_info->offsets.reset(offsets);
|
type_info->offsets.reset(offsets);
|
||||||
|
|
||||||
// Sort the fields of this message in descending order by size. We
|
|
||||||
// assume that if we then pack the fields tightly in this order, all fields
|
|
||||||
// will end up properly-aligned, since all field sizes are powers of two or
|
|
||||||
// are multiples of the system word size.
|
|
||||||
scoped_array<const FieldDescriptor*> ordered_fields(
|
|
||||||
new const FieldDescriptor*[type->field_count()]);
|
|
||||||
for (int i = 0; i < type->field_count(); i++) {
|
|
||||||
ordered_fields[i] = type->field(i);
|
|
||||||
}
|
|
||||||
stable_sort(&ordered_fields[0], &ordered_fields[type->field_count()],
|
|
||||||
DescendingFieldSizeOrder());
|
|
||||||
|
|
||||||
// Decide all field offsets by packing in order.
|
// Decide all field offsets by packing in order.
|
||||||
// We place the DynamicMessage object itself at the beginning of the allocated
|
// We place the DynamicMessage object itself at the beginning of the allocated
|
||||||
// space.
|
// space.
|
||||||
@ -501,12 +473,13 @@ const Message* DynamicMessageFactory::GetPrototype(const Descriptor* type) {
|
|||||||
type_info->extensions_offset = -1;
|
type_info->extensions_offset = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
// All the fields. We don't need to align after each field because they are
|
// All the fields.
|
||||||
// sorted in descending size order, and the size of a type is always a
|
|
||||||
// multiple of its alignment.
|
|
||||||
for (int i = 0; i < type->field_count(); i++) {
|
for (int i = 0; i < type->field_count(); i++) {
|
||||||
offsets[ordered_fields[i]->index()] = size;
|
// Make sure field is aligned to avoid bus errors.
|
||||||
size += FieldSpaceUsed(ordered_fields[i]);
|
int field_size = FieldSpaceUsed(type->field(i));
|
||||||
|
size = AlignTo(size, min(kSafeAlignment, field_size));
|
||||||
|
offsets[i] = size;
|
||||||
|
size += field_size;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add the UnknownFieldSet to the end.
|
// Add the UnknownFieldSet to the end.
|
||||||
|
Loading…
Reference in New Issue
Block a user