Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

-Wmaybe-unitialized warnings in RenderingDeviceVulkan with GCC #38829

Closed
Tungsten842 opened this issue May 18, 2020 · 6 comments · Fixed by #38835 or #40599
Closed

-Wmaybe-unitialized warnings in RenderingDeviceVulkan with GCC #38829

Tungsten842 opened this issue May 18, 2020 · 6 comments · Fixed by #38835 or #40599
Labels
bug platform:linuxbsd topic:buildsystem
Milestone
4.0

Comments

@Tungsten842
Copy link

Tungsten842 commented May 18, 2020 •
edited
Loading

Godot version:
4.0.r1.0187cda-1
OS/device including version:
arch linux intel 4500u
built with gcc version 10.1.0
Issue description:
While compiling:

[Initial build] Compiling ==> platform/linuxbsd/export/export.cpp
In file included from ./core/ustring.h:35,
                 from ./core/string_name.h:36,
                 from ./core/node_path.h:34,
                 from ./core/hashfuncs.h:36,
                 from ./core/oa_hash_map.h:34,
                 from drivers/vulkan/rendering_device_vulkan.h:34,
                 from drivers/vulkan/rendering_device_vulkan.cpp:31:
./core/cowdata.h: In member function 'VkRenderPass_T* RenderingDeviceVulkan::_render_pass_create(const Vector<RenderingDevice::AttachmentFormat>&, RenderingDevice::InitialAction, RenderingDevice::FinalAction, RenderingDevice::InitialAction, RenderingDevice::FinalAction, int*)':
./core/cowdata.h:141:3: error: '*((void*)& description +32)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  141 |   _get_data()[p_index] = p_elem;
      |   ^~~~~~~~~
drivers/vulkan/rendering_device_vulkan.cpp:2994:27: note: '*((void*)& description +32)' was declared here
 2994 |   VkAttachmentDescription description;
      |                           ^~~~~~~~~~~
In file included from ./core/ustring.h:35,
                 from ./core/string_name.h:36,
                 from ./core/node_path.h:34,
                 from ./core/hashfuncs.h:36,
                 from ./core/oa_hash_map.h:34,
                 from drivers/vulkan/rendering_device_vulkan.h:34,
                 from drivers/vulkan/rendering_device_vulkan.cpp:31:
./core/cowdata.h:141:3: error: '*((void*)& description +24)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  141 |   _get_data()[p_index] = p_elem;
      |   ^~~~~~~~~
drivers/vulkan/rendering_device_vulkan.cpp:2994:27: note: '*((void*)& description +24)' was declared here
 2994 |   VkAttachmentDescription description;
      |                           ^~~~~~~~~~~
In file included from ./core/ustring.h:35,
                 from ./core/string_name.h:36,
                 from ./core/node_path.h:34,
                 from ./core/hashfuncs.h:36,
                 from ./core/oa_hash_map.h:34,
                 from drivers/vulkan/rendering_device_vulkan.h:34,
                 from drivers/vulkan/rendering_device_vulkan.cpp:31:
./core/cowdata.h:141:3: error: '*((void*)& description +16)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  141 |   _get_data()[p_index] = p_elem;
      |   ^~~~~~~~~
drivers/vulkan/rendering_device_vulkan.cpp:2994:27: note: '*((void*)& description +16)' was declared here
 2994 |   VkAttachmentDescription description;
      |                           ^~~~~~~~~~~
[Initial build] Compiling ==> platform/osx/export/export.cpp
[Initial build] Compiling ==> platform/uwp/export/export.cpp
In file included from ./core/ustring.h:35,
                 from ./core/string_name.h:36,
                 from ./core/node_path.h:34,
                 from ./core/hashfuncs.h:36,
                 from ./core/oa_hash_map.h:34,
                 from drivers/vulkan/rendering_device_vulkan.h:34,
                 from drivers/vulkan/rendering_device_vulkan.cpp:31:
./core/cowdata.h: In member function 'void RenderingDeviceVulkan::_draw_list_insert_clear_region(RenderingDeviceVulkan::DrawList*, RenderingDeviceVulkan::Framebuffer*, Point2i, Point2i, bool, const Vector<Color>&, bool, float, uint32_t)':
./core/cowdata.h:141:3: error: 'clear_at.VkClearAttachment::clearValue.VkClearValue::color.VkClearColorValue::float32[3]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  141 |   _get_data()[p_index] = p_elem;
      |   ^~~~~~~~~
drivers/vulkan/rendering_device_vulkan.cpp:5543:21: note: 'clear_at.VkClearAttachment::clearValue.VkClearValue::color.VkClearColorValue::float32[3]' was declared here
 5543 |   VkClearAttachment clear_at;
      |                     ^~~~~~~~
In file included from ./core/ustring.h:35,
                 from ./core/string_name.h:36,
                 from ./core/node_path.h:34,
                 from ./core/hashfuncs.h:36,
                 from ./core/oa_hash_map.h:34,
                 from drivers/vulkan/rendering_device_vulkan.h:34,
                 from drivers/vulkan/rendering_device_vulkan.cpp:31:
./core/cowdata.h:141:3: error: 'clear_at.VkClearAttachment::clearValue.VkClearValue::color.VkClearColorValue::float32[2]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  141 |   _get_data()[p_index] = p_elem;
      |   ^~~~~~~~~
drivers/vulkan/rendering_device_vulkan.cpp:5543:21: note: 'clear_at.VkClearAttachment::clearValue.VkClearValue::color.VkClearColorValue::float32[2]' was declared here
 5543 |   VkClearAttachment clear_at;
      |                     ^~~~~~~~
cc1plus: all warnings being treated as errors
scons: *** [drivers/vulkan/rendering_device_vulkan.linuxbsd.opt.tools.64.o] Error 1
scons: building terminated because of errors.
==> ERROR: A failure occurred in build().
    Aborting...
@Tungsten842 Tungsten842 changed the title Compilation errors Compilation errors on arch linux May 18, 2020
@Tungsten842 Tungsten842 changed the title Compilation errors on arch linux Compilation errors on arch linux. May 18, 2020
@mkroening
Copy link

I stumbled on the same issue. This can be worked around by ignoring the warnings with the werror=no build option, but this clearly is not a proper solution.

@madmiraal madmiraal mentioned this issue May 18, 2020
Merged
@voylin
Copy link
Contributor

voylin commented Jun 16, 2020

I can confirm the exact same issue when trying to build the package on Arch Linux Kernel: 5.4.46-1-lts

@autofool
Copy link

Apparently it’s happening with all the Godot builds available in AUR repository. I just tried it first time and I love it. I really wanna work with the godot-git version but I don’t want to manually install them in system. It would be nice if this issue is resolved asap.

@akien-mga
Copy link
Member

Ask the Arch package maintainers to add werror=no to their PKGBUILD.

@Calinou
Copy link
Member

Calinou commented Jun 16, 2020 •
edited
Loading

I stumbled on the same issue. This can be worked around by ignoring the warnings with the werror=no build option, but this clearly is not a proper solution.

It's recommended to use werror=no for third-party package builds, so they don't break due to things happening during development. Not breaking the build is more important than anything at this stage 🙂

@bruvzg
Copy link
Member

bruvzg commented Jun 22, 2020

Just quick a look at relevant code:

godot/drivers/vulkan/rendering_device_vulkan.cpp

Lines 3066 to 3110 in d808697

switch (is_depth_stencil ? p_final_depth_action : p_final_color_action) {
case FINAL_ACTION_READ: {
if (p_format[i].usage_flags & TEXTURE_USAGE_COLOR_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.finalLayout = is_sampled ? VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL : (is_storage ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
} else if (p_format[i].usage_flags & TEXTURE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_STORE;
description.finalLayout = is_sampled ? VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL : (is_storage ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL);
} else {
description.loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; //don't care what is there
}
} break;
case FINAL_ACTION_DISCARD: {
if (p_format[i].usage_flags & TEXTURE_USAGE_COLOR_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.finalLayout = is_sampled ? VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL : (is_storage ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
} else if (p_format[i].usage_flags & TEXTURE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.finalLayout = is_sampled ? VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL : (is_storage ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL);
} else {
description.loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; //don't care what is there
}
} break;
case FINAL_ACTION_CONTINUE: {
if (p_format[i].usage_flags & TEXTURE_USAGE_COLOR_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.finalLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
} else if (p_format[i].usage_flags & TEXTURE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
description.storeOp = VK_ATTACHMENT_STORE_OP_STORE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_STORE;
description.finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
} else {
description.loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
description.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; //don't care what is there
}

"else" branches probably should be:

description.storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
description.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; 
description.finalLayout = VK_IMAGE_LAYOUT_UNDEFINED;

sicne loadOp/initial and filled in the earlier part of the code and storeOp/final probably are the uninitialized parts of the description.

And clearValue of the clear_at not initialized in the "if else" branch.

godot/drivers/vulkan/rendering_device_vulkan.cpp

Lines 5545 to 5561 in d808697

if (p_clear_color && texture->usage_flags & TEXTURE_USAGE_COLOR_ATTACHMENT_BIT) {
ERR_FAIL_INDEX(color_index, p_clear_colors.size()); //a bug
Color clear_color = p_clear_colors[color_index];
clear_at.clearValue.color.float32[0] = clear_color.r;
clear_at.clearValue.color.float32[1] = clear_color.g;
clear_at.clearValue.color.float32[2] = clear_color.b;
clear_at.clearValue.color.float32[3] = clear_color.a;
clear_at.colorAttachment = color_index++;
clear_at.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
} else if (p_clear_depth && texture->usage_flags & TEXTURE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
clear_at.clearValue.depthStencil.depth = p_depth;
clear_at.clearValue.depthStencil.stencil = p_stencil;
clear_at.colorAttachment = 0;
clear_at.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT;
if (format_has_stencil(texture->format)) {
clear_at.aspectMask |= VK_IMAGE_ASPECT_STENCIL_BIT;
}

Second one is probably not used anyway, first might be actual error.

@madmiraal madmiraal mentioned this issue Jun 28, 2020
Closed
@akien-mga akien-mga added the bug label Jul 13, 2020
@akien-mga akien-mga changed the title Compilation errors on arch linux. -Wmaybe-unitialized warnings in RenderingDeviceVulkan with GCC Jul 13, 2020
@akien-mga akien-mga added this to the 4.0 milestone Jul 13, 2020
@akien-mga akien-mga mentioned this issue Jul 13, 2020
Closed
akien-mga added a commit to akien-mga/godot that referenced this issue Jul 22, 2020
@akien-mga
Vulkan: Fix struct init for VkClearAttachment
dfdfee0 
The changes from godotengine#38835 were not sufficient to fix godotengine#38829, as VkClearAttachment
still had uninitialized member structs in its VkClearColor member struct.

The struct rabbit hole goes deep and trying to do validation as done in godotengine#38829
doesn't appear realistic.
@akien-mga akien-mga mentioned this issue Jul 22, 2020
Merged
MarcusElg pushed a commit to MarcusElg/godot that referenced this issue Oct 19, 2020
@akien-mga @MarcusElg
Vulkan: Fix struct init for VkClearAttachment
82525f2 
The changes from godotengine#38835 were not sufficient to fix godotengine#38829, as VkClearAttachment
still had uninitialized member structs in its VkClearColor member struct.

The struct rabbit hole goes deep and trying to do validation as done in godotengine#38829
doesn't appear realistic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug platform:linuxbsd topic:buildsystem
Projects
None yet
Milestone
4.0
8 participants
@Calinou @akien-mga @bruvzg @madmiraal @Tungsten842 @mkroening @autofool @voylin