Skip to content

Commit e674379

Browse files
committed
Fix several ubsan reported misaligned accesses
These misaligned accesses are shown in all of our CI hooks. It turned out to not be difficult to fix. It is likely that this will improve performance for aarch64.
1 parent 19e003b commit e674379

File tree

3 files changed

+10
-5
lines changed

3 files changed

+10
-5
lines changed

drivers/vulkan/rendering_device_driver_vulkan.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -3441,7 +3441,7 @@ Vector<uint8_t> RenderingDeviceDriverVulkan::shader_compile_binary_from_spirv(Ve
34413441

34423442
binary_data.shader_name_len = shader_name_utf.length();
34433443

3444-
uint32_t total_size = sizeof(uint32_t) * 3; // Header + version + main datasize;.
3444+
uint32_t total_size = sizeof(uint32_t) * 4; // Header + version + pad + main datasize;.
34453445
total_size += sizeof(ShaderBinary::Data);
34463446

34473447
total_size += STEPIFY(binary_data.shader_name_len, 4);
@@ -3470,6 +3470,8 @@ Vector<uint8_t> RenderingDeviceDriverVulkan::shader_compile_binary_from_spirv(Ve
34703470
offset += sizeof(uint32_t);
34713471
encode_uint32(sizeof(ShaderBinary::Data), binptr + offset);
34723472
offset += sizeof(uint32_t);
3473+
encode_uint32(0, binptr + offset); // Pad to align ShaderBinary::Data to 8 bytes.
3474+
offset += sizeof(uint32_t);
34733475
memcpy(binptr + offset, &binary_data, sizeof(ShaderBinary::Data));
34743476
offset += sizeof(ShaderBinary::Data);
34753477

@@ -3528,15 +3530,16 @@ RDD::ShaderID RenderingDeviceDriverVulkan::shader_create_from_bytecode(const Vec
35283530
uint32_t read_offset = 0;
35293531

35303532
// Consistency check.
3531-
ERR_FAIL_COND_V(binsize < sizeof(uint32_t) * 3 + sizeof(ShaderBinary::Data), ShaderID());
3533+
ERR_FAIL_COND_V(binsize < sizeof(uint32_t) * 4 + sizeof(ShaderBinary::Data), ShaderID());
35323534
ERR_FAIL_COND_V(binptr[0] != 'G' || binptr[1] != 'S' || binptr[2] != 'B' || binptr[3] != 'D', ShaderID());
35333535

35343536
uint32_t bin_version = decode_uint32(binptr + 4);
35353537
ERR_FAIL_COND_V(bin_version != ShaderBinary::VERSION, ShaderID());
35363538

35373539
uint32_t bin_data_size = decode_uint32(binptr + 8);
35383540

3539-
const ShaderBinary::Data &binary_data = *(reinterpret_cast<const ShaderBinary::Data *>(binptr + 12));
3541+
// 16, not 12, to skip alignment padding.
3542+
const ShaderBinary::Data &binary_data = *(reinterpret_cast<const ShaderBinary::Data *>(binptr + 16));
35403543

35413544
r_shader_desc.push_constant_size = binary_data.push_constant_size;
35423545
shader_info.vk_push_constant_stages = binary_data.vk_push_constant_stages_mask;
@@ -3549,7 +3552,7 @@ RDD::ShaderID RenderingDeviceDriverVulkan::shader_create_from_bytecode(const Vec
35493552
r_shader_desc.compute_local_size[1] = binary_data.compute_local_size[1];
35503553
r_shader_desc.compute_local_size[2] = binary_data.compute_local_size[2];
35513554

3552-
read_offset += sizeof(uint32_t) * 3 + bin_data_size;
3555+
read_offset += sizeof(uint32_t) * 4 + bin_data_size;
35533556

35543557
if (binary_data.shader_name_len) {
35553558
r_name.parse_utf8((const char *)(binptr + read_offset), binary_data.shader_name_len);

drivers/vulkan/rendering_device_driver_vulkan.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,8 @@ class RenderingDeviceDriverVulkan : public RenderingDeviceDriver {
405405
// Version 2: Added shader name.
406406
// Version 3: Added writable.
407407
// Version 4: 64-bit vertex input mask.
408-
static const uint32_t VERSION = 4;
408+
// Version 5: Add 4 bytes padding to align the Data struct after the change in version 4.
409+
static const uint32_t VERSION = 5;
409410

410411
struct DataBinding {
411412
uint32_t type = 0;

servers/rendering/rendering_device_graph.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ int32_t RenderingDeviceGraph::_add_to_write_list(int32_t p_command_index, Rect2i
228228

229229
RenderingDeviceGraph::RecordedCommand *RenderingDeviceGraph::_allocate_command(uint32_t p_command_size, int32_t &r_command_index) {
230230
uint32_t command_data_offset = command_data.size();
231+
command_data_offset = STEPIFY(command_data_offset, 8);
231232
command_data_offsets.push_back(command_data_offset);
232233
command_data.resize(command_data_offset + p_command_size);
233234
r_command_index = command_count++;

0 commit comments

Comments
 (0)