godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.34k stars 21.06k forks source link

PVS-Studio reports 2020-12-07 #44172

Closed fire closed 2 years ago

fire commented 3 years ago

Godot version: https://github.com/godotengine/godot/pull/34193/commits/287249420811c721ce502b0358d9c2068db8690b

OS/device including version: Linux Fedora 33

Issue description: Expected zero errors. But saw many errors.

image

Steps to reproduce:

cd godot
pvs-studio-analyzer credentials NAME XXXX-XXXX-XXXX-XXXX
scons p=x11 -j16
pvs-studio-analyzer analyze -f compile_commands.json  -j16
plog-converter -a GA:1,2 -t tasklist -o godot.log PVS-Studio.log -dV752,V1058
plog-converter -a GA:1,2 -t fullhtml -o html PVS-Studio.log -dV752,V1058

godot.log

html.zip

The html edition has context source code, it's easier to use.

Minimal reproduction project: Clone the Godot Engine source at commit.

fire commented 3 years ago

@qarmin You might be interested.

bruvzg commented 3 years ago

Most of (2116 of 3271) warnings are in the third party library code. These probably should be filtered out and reported to the upstream issue trackers.

fire commented 3 years ago

I will run the analyser without the thirdparty folder for comparison.

pvs-studio-analyzer analyze -f compile_commands.json  -j16 -r`pwd` -ethirdparty/

godot.log

pvs-studio-analyzer analyze -f compile_commands.json  -j16 -r`pwd` -ethirdparty/
plog-converter -a GA:1,2 -t fullhtml -o html PVS-Studio.log -dV752,V1058 -r `pwd`

html.zip

bruvzg commented 3 years ago

🔵 - Comment (code seems fine, or I do not know if it's OK). 🟡 - Duplicate checks, style, etc. Valid warning but is not critical. 🔴 - Probably actual bug.


/platform/uwp/export/export.cpp 315 warn V1001 The 'offs' variable is assigned but is not used by the end of the function. /platform/uwp/export/export.cpp 367 warn V1001 The 'offs' variable is assigned but is not used by the end of the function.

🔵 It's true, but probably done deliberately to prevent errors when new code is added to the end of function.

/editor/project_manager.cpp 2375 warn V1004 The 'EditorSettings::get_singleton()' pointer was used unsafely after it was verified against nullptr. Check lines: 2371, 2375. /editor/editor_node.cpp 5642 warn V1004 The 'EditorSettings::get_singleton()' pointer was used unsafely after it was verified against nullptr. Check lines: 5582, 5642.

🔵 Not an issue, it's calling create right after nullptr check.

/modules/gdnavigation/rvo_agent.cpp 82 warn V1004 The 'obj' pointer was used unsafely after it was verified against nullptr. Check lines: 72, 82.

🔴 Seems like actual issue, probably obj = ObjectDB::get_instance(callback.id) call should be added after callback.id = ObjectID();

/modules/gdscript/gdscript_analyzer.cpp 1610 warn V1004 The 'p_binary_op->left_operand' pointer was used unsafely after it was verified against nullptr. Check lines: 1592, 1610. /modules/gdscript/gdscript_analyzer.cpp 1610 warn V1004 The 'p_binary_op->right_operand' pointer was used unsafely after it was verified against nullptr. Check lines: 1584, 1610.

🔴 Previous code suggests these actually can be nullptr, probably something missing here, not sure what's exactly.

/modules/gridmap/grid_map_editor_plugin.cpp 939 warn V1004 The 'p_gridmap' pointer was used unsafely after it was verified against nullptr. Check lines: 909, 939.

🔴 Seems like actual issue, if it's never called with nullptr as argument, check on line 909 is wrong.

/modules/tinyexr/image_saver_tinyexr.cpp 165 warn V1009 Check the array initialization. Only the first element is initialized explicitly. The rest elements are initialized with zeros.

🔵 No idea.

/scene/resources/texture.cpp 532 warn V1015 Suspicious simultaneous use of bitwise and logical operators. /scene/resources/texture.cpp 533 warn V1015 Suspicious simultaneous use of bitwise and logical operators. /scene/resources/texture.cpp 534 warn V1015 Suspicious simultaneous use of bitwise and logical operators.

🔵 & have higher precedence than &&, if I understand this code correctly, it should work fine, but adding bracket could improve readability.

All V1016 warnings

🟡 All seems to be true, in some cases value are cast from int64_t to enum (probably check should be done before setting instead of checking value afterwards), and ERR_FAIL_INDEX_ probably should be replaced with ERR_FAIL_UNSIGNED_INDEX_.

/core/templates/command_queue_mt.h 422 warn V1020 The function exited without calling the 'unlock' function. Check lines: 422, 412.

🔵 Since it's in allocate_and_lock function, it's probably fine.

/core/io/stream_peer.cpp 186 warn V1032 The pointer 'buf' is cast to a more strictly aligned pointer type. /core/io/stream_peer.cpp 197 warn V1032 The pointer 'buf' is cast to a more strictly aligned pointer type. /core/io/stream_peer.cpp 302 warn V1032 The pointer 'buf' is cast to a more strictly aligned pointer type. /core/io/stream_peer.cpp 314 warn V1032 The pointer 'buf' is cast to a more strictly aligned pointer type.

🔵 Seems fine.

/modules/visual_script/visual_script.cpp 1553 warn V1032 The pointer 'sequence_bits' is cast to a more strictly aligned pointer type. /modules/visual_script/visual_script.cpp 1950 warn V1032 The pointer 'sequence_bits' is cast to a more strictly aligned pointer type.

    bool *sequence_bits = (bool *)(variant_stack + f->max_stack);
    const Variant **input_args = (const Variant **)(sequence_bits + f->node_count);

🟡 Not sure what's going on here, cast seems pretty strange.

/main/splash.gen.h 4 err V1043 A global object variable 'boot_splash_bg_color' is declared in the header. Multiple copies of it will be created in all translation units that include this header file. /main/splash_editor.gen.h 4 err V1043 A global object variable 'boot_splash_editor_bg_color' is declared in the header. Multiple copies of it will be created in all translation units that include this header file. /platform/android/export/gradle_export_util.h 40 err V1043 A global object variable 'godot_project_name_xml_string' is declared in the header. Multiple copies of it will be created in all translation units that include this header file.

🟡 splash.gen.h is included multiple times, thought few extra Color object aren't much of issue.

V1048 ... variable was assigned the same value.

🟡 Not sure what's better for readability.

/scene/3d/gpu_particles_3d.cpp 267 warn V1051 Consider checking for misprints. It's possible that the 'anim_material_found' should be checked here.

🔵 Seems fine, anim_material_found is checked later.

/editor/editor_settings.cpp 958 warn V1051 Consider checking for misprints. It's possible that the 'config_file_path' should be used inside 'dir->file_exists' function.

🔵 Seems fine, dir should have correct path set as current.

/platform/iphone/export/export.cpp 1545 warn V1051 Consider checking for misprints. It's possible that the 'current_dir' should be used inside 'da->change_dir' function.

🔵 Seems fine, current_dir is saved to restore it later.

/editor/plugins/visual_shader_editor_plugin.cpp 898 warn V1051 Consider checking for misprints. It's possible that the 'current_mode' should be checked here.

🔴 Something fishy here, not sure what's exactly this function supposed to do.

/scene/gui/text_edit.cpp 831 warn V1051 Consider checking for misprints. It's possible that the 'draw_amount' should be checked here. /editor/filesystem_dock.cpp 2126 warn V1051 Consider checking for misprints. It's possible that the 'drop_position' should be checked here. /editor/plugins/abstract_polygon_2d_editor.cpp 618 warn V1051 Consider checking for misprints. It's possible that the 'hover_point' should be checked here. /core/io/multiplayer_api.cpp 305 warn V1051 Consider checking for misprints. It's possible that the 'packet_len' should be checked here. /editor/plugins/animation_blend_space_1d_editor.cpp 253 warn V1051 Consider checking for misprints. It's possible that the 'point' should be checked here. /editor/editor_node.cpp 771 warn V1051 Consider checking for misprints. It's possible that the 'preset' should be checked here. /editor/plugins/visual_shader_editor_plugin.cpp 1612 warn V1051 Consider checking for misprints. It's possible that the 'prev_port' should be checked here. /editor/plugins/mesh_editor_plugin.cpp 40 warn V1051 Consider checking for misprints. It's possible that the 'rot_y' should be checked here.

🔵 Seems fine.

bruvzg commented 3 years ago

/core/io/multiplayer_api.cpp 208 err V1065 Expression can be simplified, check 'p_packet_len' and similar operands.

🔴 return p_packet_len - (p_packet_len - ofs); ???, probably should be return p_packet_len - ofs; or return ofs; ?

/modules/gdscript/gdscript_vm.cpp 2351 warn V506 Pointer to local variable 'vref' is stored outside the scope of this variable. Such a pointer will become invalid. /modules/gdscript/gdscript_vm.cpp 2682 warn V506 Pointer to local variable 'vref' is stored outside the scope of this variable. Such a pointer will become invalid.

🔵 Seems fine, it's used for variant calls.

/modules/mbedtls/packet_peer_mbed_dtls.cpp 92 warn V512 A call of the 'memcpy' function will lead to underflow of the buffer 'client_id'.

🔵 Seems fine, the rest of buffer is filled on the next line.

/core/object/object.cpp 1064 warn V519 The '_emitting' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1061, 1064.

🔵 Seems fine, it's blocking recursive calls I guess.

/modules/gdnative/gdnative.cpp 355 warn V519 The 'initialized' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 351, 355.

🔵 Seems fine, "we cheat here a little bit. you saw nothing"

/servers/rendering/renderer_rd/renderer_scene_render_rd.cpp 980 warn V519 The 'probe_half_size' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 979, 980.

    Vector3 probe_half_size = Vector3(1, 1, 1) * cascade.cell_size * float(sdfgi->cascade_size / SDFGI::PROBE_DIVISOR) * 0.5;
    probe_half_size = Vector3(0, 0, 0);

🔴 Something is wrong here, this code doesn't make any sense. Probably it was disable for debugging and left like this.

/modules/gdscript/gdscript.cpp 1376 warn V519 The 'r_ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1371, 1376.

🔵 Seems fine.

/editor/export_template_manager.cpp 230 warn V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 207, 230. /editor/editor_asset_installer.cpp 299 warn V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 254, 299. /platform/android/export/export.cpp 2760 warn V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2719, 2760.

🟡 Same ZIP code, some ret = values are not used, should be either removed of have return value checks (if (ret != UNZ_OK) ...) added.

/editor/editor_autoload_settings.cpp 230 warn V519 The 'updating_autoload' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 169, 230.

🔵 Seems fine, blocking recursive calls.

/editor/plugins/version_control_editor_plugin.cpp 116 err V522 Dereferencing of the null pointer 'EditorVCSInterface::get_singleton()' might take place.

🔵 Seems fine, singleton is created and set few lines prior to this.

/core/variant/variant_setget.cpp 1190 err V522 Dereferencing of the null pointer 'obj' might take place.

🔴 Probably check (line 1184) should be if (!obj) not if (obj != nullptr)

/drivers/vulkan/vulkan_context.cpp 385 warn V522 There might be dereferencing of a potential null pointer 'device_extensions'. Check lines: 385, 377. /drivers/vulkan/vulkan_context.cpp 239 warn V522 There might be dereferencing of a potential null pointer 'instance_extensions'. Check lines: 239, 232. /drivers/vulkan/vulkan_context.cpp 554 warn V522 There might be dereferencing of a potential null pointer 'supportsPresent'. Check lines: 554, 552.

🟡 Can be nullptr only is malloc failed.

/drivers/vulkan/vulkan_context.cpp 364 warn V522 There might be dereferencing of a potential null pointer 'physical_devices'. Check lines: 364, 357.

🟡 Code is picking first device without checking for device count. It probably OK in most cases.

/drivers/vulkan/vulkan_context.cpp 648 warn V522 There might be dereferencing of a potential null pointer 'surfFormats'. Check lines: 648, 630.

🟡 Code is assumes at least one format (which is probably always true), without checking format count.

/editor/groups_editor.cpp 186 warn V524 It is odd that the body of '_add_filter_changed' function is fully equivalent to the body of '_remove_filter_changed' function.

🟡 Two callback are identical indeed.

/drivers/unix/rw_lock_posix.cpp 64 warn V524 It is odd that the body of 'write_unlock' function is fully equivalent to the body of 'read_unlock' function.

🔵 Seems fine, virtual functions are identical in Unix implementation, but aren't in Windows one.

/servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp 1248 warn V547 Expression '!clight' is always true. /servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp 1311 warn V547 Expression '!clight' is always true. /scene/animation/animation_cache.cpp 76 warn V547 Expression '!node' is always true. /scene/animation/animation_cache.cpp 93 warn V547 Expression '!sp' is always true. /scene/animation/animation_cache.cpp 103 warn V547 Expression '!sk' is always true. /scene/animation/animation_cache.cpp 109 err V547 Expression 'idx == - 1' is always true. /modules/gdnavigation/nav_region.cpp 123 warn V547 Expression '!valid' is always true. /modules/gdscript/language_server/gdscript_workspace.cpp 124 warn V547 Expression 'err != OK' is always true. /modules/visual_script/visual_script.cpp 2129 warn V547 Expression 'function.node < 0' is always true.

🟡 Excessive checks, ERR_CONTINUE/BREAK(cond) can be replaced with ERR_PRINT(...); continue/break; or ERR_CONTINUE/BREAK(true), on the other hand printed error text might be more precise if it's left as is.

/core/os/pool_allocator.cpp 316 warn V547 Expression '!e' is always true. /core/os/pool_allocator.cpp 451 warn V547 Expression '!e' is always true. /core/os/pool_allocator.cpp 483 warn V547 Expression '!e' is always true. /core/os/pool_allocator.cpp 512 warn V547 Expression '!e' is always true. /core/os/pool_allocator.cpp 279 warn V547 Expression '!index_found' is always true. /core/os/pool_allocator.cpp 352 warn V547 Expression '!index_found' is always true. /servers/rendering/renderer_canvas_cull.cpp 1321 warn V547 Expression '!occluder_poly' is always true. /modules/enet/networked_multiplayer_enet.cpp 198 warn V547 Expression '!peer' is always true. /core/object/class_db.h 319 warn V547 Expression '!type' is always true. /editor/fileserver/editor_file_server.cpp 63 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 77 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 111 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 119 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 131 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 140 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 214 warn V547 Expression 'err != OK' is always true. /editor/fileserver/editor_file_server.cpp 224 warn V547 Expression 'err != OK' is always true. /editor/import/resource_importer_wav.cpp 199 err V547 Expression 'format_channels == 0' is always true. /editor/fileserver/editor_file_server.cpp 70 warn V547 Expression 'passlen > 512' is always true. /modules/gdscript/language_server/gdscript_language_protocol.cpp 78 err V547 Expression 'req_pos >= 4194304' is always false. /drivers/alsa/audio_driver_alsa.cpp 88 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 91 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 95 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 99 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 102 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 114 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 117 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 122 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 125 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 132 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 135 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 138 warn V547 Expression 'status < 0' is always true. /drivers/alsa/audio_driver_alsa.cpp 141 warn V547 Expression 'status < 0' is always true. /modules/gdscript/gdscript_parser.cpp 379 err V547 Expression is always true.

🟡 Excessive checks, ERR_FAIL_COND_V can be replaced with ERR_FAIL_V (inside CHECK_FAIL macro in case of alsa), on the other hand printed error text might be more precise if it's left as is.

/modules/gdscript/gdscript_tokenizer.cpp 1133 warn V547 Expression '!is_bol' is always true. /modules/gdscript/gdscript_tokenizer.cpp 1153 warn V547 Expression '!is_bol' is always true.

🔴 Something might be wrong with the logic here, is_bol is used multiple time after if (is_bol) return;

/modules/upnp/upnp.cpp 139 warn V547 Expression '!urls' is always false.

🔴 Probably should check values of urls members not the pointer itself, which is definitely not nullptr

/core/os/pool_allocator.cpp 374 warn V547 Expression '(next_pos - e->pos) > alloc_size' is always false.

🔵 Probably OK, compact_up can change e->pos.

/modules/stb_vorbis/audio_stream_ogg_vorbis.cpp 186 warn V547 Expression 'alloc_try == MAX_TEST_MEM' is always false.

🟡 Excessive check, alloc_try can't be incremented to MAX_TEST_MEM and reach this point in the same iteration.

/servers/physics_2d/space_2d_sw.cpp 623 err V547 Expression 'cbk.amount > 0' is always false. /servers/physics_3d/space_3d_sw.cpp 629 err V547 Expression 'cbk.amount > 0' is always false. /servers/physics_2d/space_2d_sw.cpp 824 warn V547 Expression 'cbk.passed > current_passed' is always false. /servers/physics_2d/space_2d_sw.cpp 838 warn V547 Expression 'did_collide' is always false.

🔵 Seems fine, cbk.amount is not set directly, but it's accessible via cbkptr pointer, which can be used to change in.

/scene/3d/audio_stream_player_3d.cpp 517 err V547 Expression 'cc >= 1' is always true.

🟡 Excessive check, cc is unsigned int.

/servers/rendering/shader_language.cpp 3793 warn V547 Expression 'cnode' is always true. /servers/rendering/shader_language.cpp 4173 warn V547 Expression 'cnode' is always true.

🟡 Excessive checks.

/core/io/multiplayer_api.cpp 918 err V547 Expression 'command_type > 7' is always false.

🟡 command_type is 1 (NETWORK_COMMAND_REMOTE_SET) or 0 (NETWORK_COMMAND_REMOTE_CALL). Maybe something else should be checked here.

/servers/rendering_server.cpp 745 err V547 Expression 'elem_size == 6' is always false.

🔴 Something wrong here sizeof(float) is 4 and neither 2 * 4 nor 3 * 4 is 6.

/platform/android/export/export.cpp 2677 err V547 Expression 'err != OK' is always false.

🔴 Probably err = is missing on one or multiple line form 2662-2675

/platform/osx/export/export.cpp 696 err V547 Expression 'err == OK' is always true.

🟡 Excessive check, probably leftover from the old code.

/scene/resources/audio_stream_sample.cpp 532 warn V547 Expression 'format == FORMAT_IMA_ADPCM' is always false.

🔵 "Saving IMA_ADPC samples are not supported yet"

/editor/plugins/visual_shader_editor_plugin.cpp 1578 warn V547 Expression 'gn != nullptr' is always true.

🟡 Excessive check, it's already checked few lines before.

/servers/rendering/renderer_rd/renderer_scene_render_rd.cpp 7380 warn V547 Expression 'half_size' is always true.

🟡 Excessive check, hardcoded to bool half_size = true; //much faster, very little difference

/modules/gdscript/language_server/gdscript_language_protocol.cpp 73 warn V547 Expression 'has_header' is always true.

🟡 Excessive check, won't reach this point if has_header is not true.

/scene/main/viewport.cpp 1827 warn V547 Expression 'is_handled' is always false.

    bool is_handled = false;
    if (is_handled) {
        set_input_as_handled();
        return;
    }

🔴 Something is missing here.

/scene/2d/sprite_2d.cpp 328 warn V547 Expression 'is_repeat' is always false.

🟡 Not reimplemented: "This need to be obtained from CanvasItem new repeat mode (but it needs to guess it from hierarchy, need to add a function for that)"

/modules/bullet/shape_bullet.cpp 518 warn V547 Expression 'l_depth <= 0' is always false. /modules/bullet/shape_bullet.cpp 517 warn V547 Expression 'l_width <= 0' is always false.

🟡 Excessive check, both are previously checked to be at least 2.

/core/io/multiplayer_api.cpp 920 err V547 Expression 'name_id_compression > 1' is always false. /core/io/multiplayer_api.cpp 919 err V547 Expression 'node_id_compression > 3' is always false.

🟡 Excessive check, both are always set to the valid value.

/scene/gui/popup_menu.cpp 124 warn V547 Expression 'p_item < 0' is always false.

🟡 Excessive check, already checked by ERR_FAIL_INDEX.

/core/io/multiplayer_api.cpp 239 warn V547 Expression 'p_packet_len < packet_min_size' is always false.

🟡 Excessive check, at this point packet_min_size is 1, and p_packet_len < 1 already checked before.

/core/os/memory.cpp 76 warn V547 Expression 'prepad' is always true. /core/os/memory.cpp 82 warn V547 Expression 'prepad' is always true. /core/os/memory.cpp 111 warn V547 Expression 'prepad' is always true. /core/os/memory.cpp 161 warn V547 Expression 'prepad' is always true.

🔵 prepad is always true only in debug build.

/core/object/object.cpp 536 warn V547 Expression 'r_valid' is always true. /core/object/object.cpp 552 warn V547 Expression 'r_valid' is always true.

🟡 Excessive checks, r_valid is either valid external pointer, or pointer to local variable valid.

/core/io/compression.cpp 266 err V547 Expression 'ret == 1' is always true. /core/io/compression.cpp 260 warn V560 A part of conditional expression is always true: ret == 1.

🟡 Excessive check, it's exit condition of previous loop.

/main/main.cpp 1631 warn V547 Expression 'show_logo' is always true.

🔵 Value is hardcoded base on JAVASCRIPT_ENABLED.

/scene/resources/visual_shader_nodes.cpp 994 err V547 Expression 'source == SOURCE_PORT' is always true. /scene/resources/visual_shader_nodes.cpp 1205 err V547 Expression 'source == SOURCE_PORT' is always true.

🔵 Not sure why it's reporting source == SOURCE_PORT always true.

/servers/rendering/renderer_rd/renderer_storage_rd.cpp 8363 warn V547 Expression 'using_lightmap_array' is always true.

using_lightmap_array = true; // high end
if (using_lightmap_array) {

🟡 Temporary hardcoded, I guess.

/scene/gui/split_container.cpp 262 warn V547 Expression 'vertical' is always false.

🟡 Excessive check, should be split_offset = drag_ofs + (drag_from - mm->get_position().x);

/modules/gdscript/gdscript_editor.cpp 2373 err V547 Expression is always false. /modules/gdscript/gdscript_editor.cpp 2366 warn V560 A part of conditional expression is always false. /modules/gdscript/gdscript_analyzer.cpp 1965 warn V560 A part of conditional expression is always true: is_self.

🔴 Something wrong here callee_type is set to GDScriptParser::Node::NONE and never change.

/servers/physics_2d/body_2d_sw.cpp 314 warn V547 Expression is always true.

🟡 Excessive (mode != PhysicsServer2D::BODY_MODE_STATIC) check, there's if (mode == PhysicsServer2D::BODY_MODE_STATIC || before

/servers/physics_2d/joints_2d_sw.cpp 173 err V547 Expression is always true. /servers/physics_2d/joints_2d_sw.cpp 179 err V547 Expression is always true.

🔵 Why?

bruvzg commented 3 years ago

/platform/uwp/export/export.cpp 482 warn V555 The expression 'p_len - step > 0' will work as 'p_len != step'. /editor/debugger/editor_visual_profiler.cpp 71 warn V555 The expression of the 'A - B > 0' kind will work as 'A != B'.

🔵 And what the issue with that?

/core/math/face3.cpp 101 warn V557 Array overrun is possible. The value of 'polygons_created' index could reach 3. /core/math/face3.cpp 102 warn V557 Array overrun is possible. The value of 'polygons_created' index could reach 3.

🔵 Seems ok, both above_count and below_count can't be 4 at the same time.

/scene/resources/style_box.cpp 771 warn V560 A part of conditional expression is always false: !draw_border.

🔵 Why it's false?

V560 A part of conditional expression is always false: (int)p_text_direction

🔵 True if it was used form C++ code only, but since it's exposed to GDScript and it will case any int value to enum, this checks are valid (e.g. following scrip will trigger error message $Button.text_direction = 20200)

/editor/plugins/editor_preview_plugins.cpp 533 warn V560 A part of conditional expression is always false: c == '\t'. /editor/plugins/editor_preview_plugins.cpp 533 warn V560 A part of conditional expression is always true: c >= '!'.

🟡 There's if (c > 32) check before and \t is 9 and '!' is 33

/servers/physics_2d/space_2d_sw.cpp 827 warn V560 A part of conditional expression is always false: cbk.invalid_by_dir > 0. /servers/physics_2d/space_2d_sw.cpp 827 warn V560 A part of conditional expression is always true: !did_collide.

🔵 It's fine, cbk is accessible via pointer and can be changed by CollisionSolver2DSW::solve.

/core/io/marshalls.cpp 618 warn V560 A part of conditional expression is always false: count < 0. /core/io/marshalls.cpp 666 warn V560 A part of conditional expression is always false: count < 0. /core/io/marshalls.cpp 743 warn V560 A part of conditional expression is always false: count < 0. /core/io/marshalls.cpp 776 warn V560 A part of conditional expression is always false: count < 0. /core/io/marshalls.cpp 811 warn V560 A part of conditional expression is always false: count < 0.

🟡 count < 0 is checked in ERR_FAIL_MUL_OF

/core/variant/array.cpp 343 warn V560 A part of conditional expression is always false: dest_idx < 0. /core/variant/array.cpp 349 warn V560 A part of conditional expression is always false: dest_idx < 0.

🟡 dest_idx starts at 0, and is only incremented.

/scene/gui/tabs.cpp 699 warn V560 A part of conditional expression is always false: i == current.

🟡 It's inside if (... && i != current)

/modules/gdscript/gdscript_editor.cpp 2277 warn V560 A part of conditional expression is always false: method_args > 0.

🔵 No, it can be set to non zero value (line 2236).

/editor/plugins/node_3d_editor_plugin.cpp 2142 warn V560 A part of conditional expression is always false: orthogonal.

🟡 There's if (orthogonal) { ... return; } check before.

/modules/webrtc/webrtc_multiplayer.cpp 246 warn V560 A part of conditional expression is always false: p_peer_id > ~(1 << 31). /modules/webrtc/webrtc_multiplayer.cpp 196 warn V560 A part of conditional expression is always false: p_self_id > ~(1 << 31).

🟡 It's 32-bit signed int, check is excessive.

/core/io/marshalls.cpp 79 warn V560 A part of conditional expression is always false: strlen < 0.

🟡 Already checked by ERR_FAIL_ADD_OF

/main/main.cpp 2349 warn V560 A part of conditional expression is always true: (!hasicon).

🔵 No, it's not on macOS and Windows.

/core/io/json.cpp 221 warn V560 A part of conditional expression is always true: c <= 'f'. /core/io/json.cpp 224 warn V560 A part of conditional expression is always true: c <= 'F'. /core/variant/variant_parser.cpp 258 warn V560 A part of conditional expression is always true: c <= 'f'. /core/variant/variant_parser.cpp 261 warn V560 A part of conditional expression is always true: c <= 'F'. /modules/visual_script/visual_script_expression.cpp 385 warn V560 A part of conditional expression is always true: c <= 'f'. /modules/visual_script/visual_script_expression.cpp 388 warn V560 A part of conditional expression is always true: c <= 'F'. /core/io/json.cpp 219 warn V560 A part of conditional expression is always true: c >= '0'. /core/variant/variant_parser.cpp 256 warn V560 A part of conditional expression is always true: c >= '0'. /modules/visual_script/visual_script_expression.cpp 383 warn V560 A part of conditional expression is always true: c >= '0'. /core/io/json.cpp 224 warn V560 A part of conditional expression is always true: c >= 'A'. /core/variant/variant_parser.cpp 261 warn V560 A part of conditional expression is always true: c >= 'A'. /modules/visual_script/visual_script_expression.cpp 388 warn V560 A part of conditional expression is always true: c >= 'A'. /platform/android/export/export.cpp 508 warn V560 A part of conditional expression is always true: c >= '0'.

🟡 Already checked to be in range few lines before

/servers/rendering/renderer_rd/renderer_scene_render_rd.cpp 6534 warn V560 A part of conditional expression is always true: env->volumetric_fog_enabled. /servers/rendering/renderer_rd/renderer_scene_render_rd.cpp 6534 warn V560 A part of conditional expression is always true: env.

🟡 There's if (!env || !env->volumetric_fog_enabled) { return } right before it.

/platform/linuxbsd/joypad_linux.cpp 507 warn V560 A part of conditional expression is always true: len < 0.

🟡 Excessive check, len <= 0 is exit condition of while loop before it.

/modules/gdscript/gdscript_function.h 114 warn V560 A part of conditional expression is always true: obj.

🟡 Already checked to be non null.

/core/string/ustring.cpp 2132 warn V560 A part of conditional expression is always true: sign == - 1. /core/string/ustring.cpp 2173 warn V560 A part of conditional expression is always true: sign == - 1.

🟡 sign == 1 || (sign == -1 && ...) seems excessive, it's -1 or 1.

/servers/rendering/renderer_rd/renderer_scene_render_forward.cpp 1510 err V570 The 'e->shader_index' variable is assigned to itself.

🔴 Not sure what's should be here, maybe e->shader_index = e->material->shader_data->index?

/drivers/vulkan/rendering_device_vulkan.cpp 4105 err V570 The 'info.binding' variable is assigned to itself.

🔴 Hm?

/drivers/png/png_driver_common.cpp 117 err V575 The null pointer is passed into 'png_image_finish_read' function. Inspect the fifth argument. /drivers/png/png_driver_common.cpp 117 err V575 The null pointer is passed into 'png_image_finish_read' function. Inspect the second argument. /drivers/png/png_driver_common.cpp 181 err V575 The null pointer is passed into 'png_image_write_to_memory' function. Inspect the seventh argument. /drivers/png/png_driver_common.cpp 194 err V575 The null pointer is passed into 'png_image_write_to_memory' function. Inspect the seventh argument.

🔵 Seems fine, it's optional arguments.

/core/variant/dictionary.cpp 161 err V595 The '_p' pointer was utilized before it was verified against nullptr. Check lines: 161, 164. /core/templates/rid_owner.h 304 err V595 The 'chunks' pointer was utilized before it was verified against nullptr. Check lines: 304, 316. /platform/linuxbsd/display_server_x11.cpp 4282 err V595 The 'context_vulkan' pointer was utilized before it was verified against nullptr. Check lines: 4282, 4303. /scene/main/viewport.cpp 2853 err V595 The 'gui.subwindow_focused' pointer was utilized before it was verified against nullptr. Check lines: 2853, 2856. /servers/rendering/renderer_rd/renderer_storage_rd.cpp 2786 err V595 The 'mesh->surfaces' pointer was utilized before it was verified against nullptr. Check lines: 2786, 2788. /servers/rendering/renderer_rd/renderer_storage_rd.cpp 6397 warn V614 Potentially uninitialized variable 'scale' used. /modules/text_server_adv/dynamic_font_adv.cpp 442 err V616 The '(FT_RENDER_MODE_NORMAL)' named constant with the value of 0 is used in the bitwise operation. /core/io/image.cpp 2145 err V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. /servers/physics_2d/space_2d_sw.cpp 642 err V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. /servers/physics_3d/space_3d_sw.cpp 648 err V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. /core/io/image.cpp 2145 err V654 The condition 'i < pixel_size' of loop is always false. /servers/physics_2d/space_2d_sw.cpp 642 err V654 The condition 'k < cbk.amount' of loop is always false. /servers/physics_3d/space_3d_sw.cpp 648 err V654 The condition 'k < cbk.amount' of loop is always false.

🔵 Seems fine.

bruvzg commented 3 years ago

@bruvzg Can you try to see if #44172 (comment) 's html zip works?

I tried it on windows and I generated the zip from Fedora 33.

One form the first post works file, and the second one have some files packed twice (index in the root is not working due to missing files, but index in the fullhtml subfolder do).

fire commented 3 years ago

Reporting for clarity.

fullhtml.zip

fire commented 3 years ago

If there are no ojections I'll close this issue for https://github.com/godotengine/godot/issues/44240

akien-mga commented 3 years ago

Well there's a ton of feedback and research in this issue, why create a new one for the same?

fire commented 3 years ago

Because I think the issue would be more readable to remove the finished bugs from the list and copy the entire list to a new list.

It's not a big deal. So wanted to ask.

bruvzg commented 3 years ago

I have removed all fixed bugs from the previous comments (it's still visible in the edit history).

fire commented 2 years ago

I'll start a new report. This PVS-Studio report has been almost a year.