Closed akien-mga closed 2 years ago
Just to confirm am aware and will fix as soon as I get GCC12 installed. :grin:
From what I can see this seems a warning caused because the client input at GodotBroadPhase2DBVH
is not checked, if a garbage input was passed to the BVH from the calling code it could cause a crash.
The reason the checks in the BVH are DEV_ENABLED
only is because it is hot bottleneck code (it could severely slow down physics etc), but we can probably put some ERR_FAIL
s in the calling code / API to prevent the warning firing off without much effect on performance. The fact the DEV asserts aren't going off indicates this isn't actually a "live bug" in practice.
Still compiling (5 hours later lol), but I don't seem to be getting this warning at least building the editor with release_debug
and warnings=extra
, with GCC 12.2.1 (compiled from source).
I followed this guide: https://stackoverflow.com/questions/70835585/how-to-install-gcc-12-on-ubuntu (Although on Linux Mint 20). Confirmed it was compiling with GCC 12.
I'll report whether it shows the warning with command line shown in the steps to reproduce, it terminated before the build was complete due to another warning.
EDIT: Yes it does occur later in the build, no idea why it doesn't appear in the editor build warnings:
scons p=linuxbsd tools=no target=release warnings=extra werror=no
.. could DEV_ENABLED
be enabled during release_debug
editor builds?
5 hours? That's really weird.
You can't build the editor with target=release
, only the template (which is where I saw the warning). I don't think the warning happens with target=release_debug
, which probably indicates that it happens with -O3
optimization (release
) but not with -O2
(release_debug
).
5 hours? That's really weird.
You can't build the editor with
target=release
, only the template (which is where I saw the warning). I don't think the warning happens withtarget=release_debug
, which probably indicates that it happens with-O3
optimization (release
) but not with-O2
(release_debug
).
Ah could be the optimization level. Yes most of that was compiling GCC from source though and me figuring out how to get scons to use it. :grinning:
OK I think I've figured out why it's tripping, and can confirm is a false flag:
_node_add_item()
calls:
ref.item_id = leaf.request_item();
BVH_ASSERT(ref.item_id != BVHCommon::INVALID);
// set the aabb of the new item
leaf.get_aabb(ref.item_id) = p_aabb;
The request_item()
could theoretically return -1 (for failing), which would then be passed to leaf.get_aabb()
and read out of bounds of the array[128]. However, the logic outside of _node_add_item()
ensures that there is room (i.e. request_item()
will not fail). This logic is checked with the DEV_ASSERT
in case of changing the logic outside the routine causing this error.
We can probably eliminate the warning by returning 0 instead of -1 in the case of non-DEV_ENABLED builds, purely to silence the warning.
I'll double check this can't happen, but I'm pretty sure this is the reason for the warning. It kind of makes sense, but the compiler probably can't account for complex logic from the calling routine.
Indeed a little search indicates this particular type of warning has a lot of bugs: https://stackoverflow.com/questions/72871100/compiler-library-or-user-error-eigenarray-gcc-12-1-array-subscript
EDIT: Yes can confirm swapping return value to 0 fixes the warning, and it should never occur at runtime due to the _logic_choose_item_add_node()
creating a new node if the current one is full. Will create a PR. :+1:
Godot version
4.0.beta (8e14f9ba21725a9445f3979742f2fc87c8a7b463)
System information
Mageia 9 Linux, x86_64 | GCC 12.2.1 (20220917)
Issue description
When compiling on Linux with GCC 12.2.1 (snapshot from 20220917) with warnings enabled (and
werror=yes
to treat them as errors), some-Werror=array-bounds
warnings are raised in BVH structs. The warnings aren't raised by GCC 10.2.0 - I didn't test with GCC 11.x recently but I remember seeing those warnings for a while so probably any GCC 12.x version might raise them.Steps to reproduce
scons p=linuxbsd tools=no target=release warnings=extra werror=yes
Minimal reproduction project
No response