[rcore] UnloadShader(), new shader locations are not properly freed when shader fallback to default

[nagolove]

• [ OK ] I tested it on latest raylib version from master branch
• [ OK] I checked there is no similar issue already reported
• [ OK] I checked the documentation on the wiki
• [ OK ] My code has no errors or misuse of raylib

Issue description

Raylib has memory leak in Shader unloading. When it could not load shader library put default shaders descriptors to creating structure. But when I unload this shader locs array is not freed.

raylib/src/rcore.c

Lines 1411 to 1420 in 7ecc47d

	void UnloadShader(Shader shader)
	{
	if (shader.id != rlGetShaderIdDefault())
	{
	rlUnloadShaderProgram(shader.id);
	// NOTE: If shader loading failed, it should be 0
	RL_FREE(shader.locs);
	}
	}

==72430==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 128 byte(s) in 1 object(s) allocated from:
    #0 0x7e22aaefd1aa in calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x5c509a18ccf2 in LoadShaderFromMemory /home/nagolove/projects/raylib_last/src/rcore.c:1344
    #2 0x5c509a18cc6f in LoadShader /home/nagolove/projects/raylib_last/src/rcore.c:1314
    #3 0x5c509a15d999 in main (/home/nagolove/raylib_unload_shader/a.out+0x8999) (BuildId: 12b518d97cdb46792438c286ae74a0b90d53dead)
    #4 0x7e22aa434e07  (/usr/lib/libc.so.6+0x25e07) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #5 0x7e22aa434ecb in __libc_start_main (/usr/lib/libc.so.6+0x25ecb) (BuildId: 98b3d8e0b8c534c769cb871c438b4f8f3a8e4bf3)
    #6 0x5c509a15d824 in _start (/home/nagolove/raylib_unload_shader/a.out+0x8824) (BuildId: 12b518d97cdb46792438c286ae74a0b90d53dead)

SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s).

Environment

OS: Arch Linux x86_64
Raylib version: b079679

Code Example

#include "raylib.h"
#include <stdlib.h>

int main(void) {
    InitWindow(800, 600, "raylib [shaders] example");
    Shader shader = LoadShader(0, "unavaible_path/unknown.fs");

    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(WHITE);
        EndDrawing();
    }

    UnloadShader(shader);               // Unload shader

// workaround
#if 0
    if (shader.locs) {
        free(shader.locs);
        shader.locs = NULL;
    }
#endif

    CloseWindow();                      // Close window and OpenGL context
    return 0;
}

Build command

gcc main.c -lm -lraylib -fsanitize=undefined,address 

[raysan5]

@nagolove Good catch! Thanks for reporting! Just added a quick fix to make sure the default shader locations are returned in case of default shader fallback... but default shader fallback was supposed to be removed in latest raylib 5.5 release! Other fallback mechanisms have already been removed in the last few years (models, fonts, textures).

I'm keeping this issue open while I review the full shader system because avoiding fallback could generate some crash.

[raysan5]

I've been further reviewing this issue and it's a bit tricky, in case of shader loading failure, current implementation fallsback to default shader. This is the easier solution because of shader.locs is initialized while on case of error it is NULL.

shader.locs is accessed by the following critical functions:

• rlgl: rlLoadRenderBatch(), rlDrawRenderbatch()
• rmodels: DrawMesh(), DrawMeshInstanced()

In case (shader.locs == NULL), all those functions need to be reviewed to avoid setting the different uniform locations, increasing code considerably; note that they can not just if (shader.locs == NULL) return; because it will imply the meshes not drawing.

I think in this case it's just better to fallback to a default shader.