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.

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

Accessing a null MeshInstance object in BakedLightmap instead of the GeometryInstance #57109

Closed
Macksaur opened this issue Jan 24, 2022 · 1 comment

Comments

@Macksaur
Copy link
Contributor

Godot version

3.x

System information

Linux

Issue description

mi has been checked/tested to be null, yet is accessed on the following lines:

all_override = mi->get_material_override();

mf.cast_shadows = mi->get_cast_shadows_setting() != GeometryInstance::SHADOW_CASTING_SETTING_OFF;

mf.generate_lightmap = mi->get_generate_lightmap();

Should these variables be referencing gi?

Steps to reproduce

Compile with -Wnonnull. 4.0 doesn't look to be affected.

The warning is somewhat hard to trigger due to the UB involved.
Using the following command scons -j16 platform=x11 tools=no target=release_debug CCFLAGS=-Wnonnull after touching scene/3d/baked_lightmap.cpp produces (for me):

scene/3d/baked_lightmap.cpp: In member function 'void BakedLightmap::_find_meshes_and_lights(Node*, Vector<BakedLightmap::MeshesFound>&, Vector<BakedLightmap::LightsFound>&)':
scene/3d/baked_lightmap.cpp:392:73: warning: 'this' pointer is null [-Wnonnull]
  392 |                                 all_override = mi->get_material_override();
      |                                                ~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from scene/3d/multimesh_instance.h:34,
                 from scene/3d/baked_lightmap.h:35,
                 from scene/3d/baked_lightmap.cpp:31:
./scene/3d/visual_instance.h:151:23: note: in a call to non-static member function 'Ref<Material> GeometryInstance::get_material_override() const'
  151 |         Ref<Material> get_material_override() const;
      |                       ^~~~~~~~~~~~~~~~~~~~~
scene/3d/baked_lightmap.cpp:417:87: warning: 'this' pointer is null [-Wnonnull]
  417 |                                         mf.cast_shadows = mi->get_cast_shadows_setting() != GeometryInstance::SHADOW_CASTING_SETTING_OFF;

      |                                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from scene/3d/multimesh_instance.h:34,
                 from scene/3d/baked_lightmap.h:35,
                 from scene/3d/baked_lightmap.cpp:31:
./scene/3d/visual_instance.h:130:30: note: in a call to non-static member function 'GeometryInstance::ShadowCastingSetting GeometryInstance::get_cast_shadows_setting() const'
  130 |         ShadowCastingSetting get_cast_shadows_setting() const;
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~
scene/3d/baked_lightmap.cpp:418:89: warning: 'this' pointer is null [-Wnonnull]
  418 |                                         mf.generate_lightmap = mi->get_generate_lightmap();
      |                                                                ~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from scene/3d/multimesh_instance.h:34,
                 from scene/3d/baked_lightmap.h:35,
                 from scene/3d/baked_lightmap.cpp:31:
./scene/3d/visual_instance.h:133:14: note: in a call to non-static member function 'bool GeometryInstance::get_generate_lightmap()'
  133 |         bool get_generate_lightmap();
      |              ^~~~~~~~~~~~~~~~~~~~~

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Jan 24, 2022

Fixed by #57110. (This issue wasn't closed because the pull request was merged on 3.x, which isn't this repository's default branch.)

@Calinou Calinou closed this as completed Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants