Fix crash in pipeline init when passing invalid parameter

SanderMertens · Sep 14, 2023 · 9365681 · 9365681
1 parent ee3224f
commit 9365681
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 11 deletions.
diff --git a/flecs.c b/flecs.c
@@ -58872,7 +58872,7 @@ ecs_entity_t ecs_pipeline_init(
     const ecs_pipeline_desc_t *desc)
 {
     ecs_poly_assert(world, ecs_world_t);
-    ecs_assert(desc != NULL, ECS_INVALID_PARAMETER, NULL);
+    ecs_check(desc != NULL, ECS_INVALID_PARAMETER, NULL);
 
     ecs_entity_t result = desc->entity;
     if (!result) {
@@ -58891,8 +58891,10 @@ ecs_entity_t ecs_pipeline_init(
         return 0;
     }
 
-    ecs_assert(query->filter.terms[0].id == EcsSystem,
-        ECS_INVALID_PARAMETER, NULL);
+    ecs_check(query->filter.terms != NULL, ECS_INVALID_PARAMETER, 
+        "pipeline query cannot be empty");
+    ecs_check(query->filter.terms[0].id == EcsSystem,
+        ECS_INVALID_PARAMETER, "pipeline must start with System term");
 
     ecs_pipeline_state_t *pq = ecs_os_calloc_t(ecs_pipeline_state_t);
     pq->query = query;
@@ -58901,6 +58903,8 @@ ecs_entity_t ecs_pipeline_init(
     ecs_set(world, result, EcsPipeline, { pq });
 
     return result;
+error:
+    return 0;
 }
 
 /* -- Module implementation -- */

diff --git a/src/addons/pipeline/pipeline.c b/src/addons/pipeline/pipeline.c
@@ -805,7 +805,7 @@ ecs_entity_t ecs_pipeline_init(
     const ecs_pipeline_desc_t *desc)
 {
     ecs_poly_assert(world, ecs_world_t);
-    ecs_assert(desc != NULL, ECS_INVALID_PARAMETER, NULL);
+    ecs_check(desc != NULL, ECS_INVALID_PARAMETER, NULL);
 
     ecs_entity_t result = desc->entity;
     if (!result) {
@@ -824,8 +824,10 @@ ecs_entity_t ecs_pipeline_init(
         return 0;
     }
 
-    ecs_assert(query->filter.terms[0].id == EcsSystem,
-        ECS_INVALID_PARAMETER, NULL);
+    ecs_check(query->filter.terms != NULL, ECS_INVALID_PARAMETER, 
+        "pipeline query cannot be empty");
+    ecs_check(query->filter.terms[0].id == EcsSystem,
+        ECS_INVALID_PARAMETER, "pipeline must start with System term");
 
     ecs_pipeline_state_t *pq = ecs_os_calloc_t(ecs_pipeline_state_t);
     pq->query = query;
@@ -834,6 +836,8 @@ ecs_entity_t ecs_pipeline_init(
     ecs_set(world, result, EcsPipeline, { pq });
 
     return result;
+error:
+    return 0;
 }
 
 /* -- Module implementation -- */

diff --git a/test/addons/project.json b/test/addons/project.json
@@ -514,7 +514,7 @@
                 "random_in_after_random_inout_after_random_out",
                 "stage_write_before_read",
                 "mixed_multithreaded",
-		"mixed_multithreaded_tasks",
+		        "mixed_multithreaded_tasks",
                 "mixed_staging",
                 "no_staging_system_create_query",
                 "single_threaded_pipeline_change",
@@ -528,13 +528,13 @@
                 "stack_allocator_after_progress",
                 "stack_allocator_after_progress_w_pipeline_change",
                 "iter_from_world_in_singlethread_system_multitead_app",
-		"iter_from_world_in_singlethread_system_multitead_app_tasks",
+		        "iter_from_world_in_singlethread_system_multitead_app_tasks",
                 "no_staging_after_inactive_system",
                 "inactive_system_after_no_staging_system_no_defer_w_filter",
                 "inactive_system_after_2_no_staging_system_no_defer_w_filter",
                 "inactive_system_after_no_staging_system_no_defer_w_filter_w_no_staging_at_end",
                 "inactive_multithread_system_after_no_staging_system_no_defer",
-		"inactive_multithread_tasks_system_after_no_staging_system_no_defer",
+		        "inactive_multithread_tasks_system_after_no_staging_system_no_defer",
                 "inactive_multithread_system_after_no_staging_system_no_defer_w_no_staging_at_end",
                 "inactive_multithread_tasks_system_after_no_staging_system_no_defer_w_no_staging_at_end",
                 "multi_threaded_pipeline_change_w_only_singlethreaded",
@@ -562,7 +562,9 @@
                 "switch_from_threads_to_tasks",
                 "switch_from_tasks_to_threads",
                 "run_pipeline_multithreaded",
-                "run_pipeline_multithreaded_tasks"
+                "run_pipeline_multithreaded_tasks",
+                "pipeline_init_no_terms",
+                "pipeline_init_no_system_term"
             ]
         }, {
             "id": "SystemMisc",

diff --git a/test/addons/src/Pipeline.c b/test/addons/src/Pipeline.c
@@ -3184,3 +3184,25 @@ void Pipeline_run_pipeline_multithreaded(void) {
 void Pipeline_run_pipeline_multithreaded_tasks(void) {
     Pipeline_run_pipeline_multithreaded_internal(true);
 }
+
+void Pipeline_pipeline_init_no_terms(void) {
+    install_test_abort();
+
+    ecs_world_t *world = ecs_init();
+
+    test_expect_abort();
+    ecs_pipeline(world, { 0 });
+}
+
+void Pipeline_pipeline_init_no_system_term(void) {
+    install_test_abort();
+
+    ecs_world_t *world = ecs_init();
+
+    ECS_COMPONENT(world, Position);
+
+    test_expect_abort();
+    ecs_pipeline(world, {
+        .query.filter.terms = {{ ecs_id(Position) }}
+    });
+}
diff --git a/test/addons/src/main.c b/test/addons/src/main.c
@@ -552,6 +552,8 @@ void Pipeline_switch_from_threads_to_tasks(void);
 void Pipeline_switch_from_tasks_to_threads(void);
 void Pipeline_run_pipeline_multithreaded(void);
 void Pipeline_run_pipeline_multithreaded_tasks(void);
+void Pipeline_pipeline_init_no_terms(void);
+void Pipeline_pipeline_init_no_system_term(void);
 
 // Testsuite 'SystemMisc'
 void SystemMisc_invalid_not_without_id(void);
@@ -3695,6 +3697,14 @@ bake_test_case Pipeline_testcases[] = {
     {
         "run_pipeline_multithreaded_tasks",
         Pipeline_run_pipeline_multithreaded_tasks
+    },
+    {
+        "pipeline_init_no_terms",
+        Pipeline_pipeline_init_no_terms
+    },
+    {
+        "pipeline_init_no_system_term",
+        Pipeline_pipeline_init_no_system_term
     }
 };
 
@@ -7477,7 +7487,7 @@ static bake_test_suite suites[] = {
         "Pipeline",
         NULL,
         NULL,
-        80,
+        82,
         Pipeline_testcases
     },
     {