From 8b50e0e61840560ec2f8da7d36561dc70fecef63 Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Tue, 26 Aug 2025 17:20:08 +0200 Subject: [PATCH 1/3] update: move issue #401 to DOING --- BACKLOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 24ac1bcf..a54deb2d 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -4,7 +4,6 @@ **🚨 CRITICAL: CI test suite failures - user-visible rendering broken** -- [ ] #401: Fix - CI test-coverage failing with segfaults in colored_contours and ascii_heatmap_demo (blocks all PR merges) **🚨 CRITICAL: User-visible PNG/PDF rendering regressions** @@ -15,7 +14,7 @@ - [ ] #388: Fix - investigate test_mpeg_consolidated failure unrelated to ylabel rotation (test infrastructure) ## DOING (Current Work) -*Empty - ready for next issue* +- [x] #401: Fix - CI test-coverage failing with segfaults in colored_contours and ascii_heatmap_demo (branch: fix-ci-segfaults-401) ## FUTURE SPRINTS - Systematic Restoration From 84081c32c52c2c7740d171d2e0f2aff253f3ced3 Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Tue, 26 Aug 2025 17:25:48 +0200 Subject: [PATCH 2/3] fix: resolve CI segfaults in contour plotting functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace unsafe merge() function usage in matplotlib wrapper with conditional calls to prevent memory access violations. The merge() function was causing segfaults when optional parameters were not present, particularly in colored_contours and ascii_heatmap_demo examples. Memory safety improvements: - Replace merge() with explicit conditional parameter forwarding - Fix both contour_filled() and add_contour_filled() functions - Prevent passing uninitialized zero-size arrays to underlying methods - Add comprehensive regression tests to prevent future occurrences Fixes critical CI blocking issue where examples crashed with SIGSEGV. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/fortplot_matplotlib.f90 | 84 +++++++++++-- .../test_contour_memory_safety_regression.f90 | 113 ++++++++++++++++++ 2 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 test/test_contour_memory_safety_regression.f90 diff --git a/src/fortplot_matplotlib.f90 b/src/fortplot_matplotlib.f90 index 0e81d64b..3fa8d7d6 100644 --- a/src/fortplot_matplotlib.f90 +++ b/src/fortplot_matplotlib.f90 @@ -108,12 +108,42 @@ subroutine contour_filled(x, y, z, levels, colormap, show_colorbar, label) allocate(wp_levels(0)) end if - ! Forward ALL parameters to underlying method using single call pattern - call fig%add_contour_filled(wp_x, wp_y, wp_z, & - levels=merge(wp_levels, wp_levels, present(levels)), & - colormap=merge(colormap, "", present(colormap)), & - show_colorbar=merge(show_colorbar, .false., present(show_colorbar)), & - label=merge(label, "", present(label))) + ! Forward parameters to underlying method using conditional calls for memory safety + if (present(levels) .and. present(colormap) .and. present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, show_colorbar=show_colorbar, label=label) + else if (present(levels) .and. present(colormap) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, show_colorbar=show_colorbar) + else if (present(levels) .and. present(colormap) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, label=label) + else if (present(colormap) .and. present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, & + show_colorbar=show_colorbar, label=label) + else if (present(levels) .and. present(colormap)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, colormap=colormap) + else if (present(levels) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, show_colorbar=show_colorbar) + else if (present(levels) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, label=label) + else if (present(colormap) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, show_colorbar=show_colorbar) + else if (present(colormap) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, label=label) + else if (present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, show_colorbar=show_colorbar, label=label) + else if (present(levels)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels) + else if (present(colormap)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap) + else if (present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, show_colorbar=show_colorbar) + else if (present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, label=label) + else + call fig%add_contour_filled(wp_x, wp_y, wp_z) + end if deallocate(wp_x, wp_y, wp_z) if (allocated(wp_levels)) deallocate(wp_levels) @@ -433,12 +463,42 @@ subroutine add_contour_filled(x, y, z, levels, colormap, show_colorbar, label) allocate(wp_levels(0)) end if - ! Forward ALL parameters to underlying method using single call pattern - call fig%add_contour_filled(wp_x, wp_y, wp_z, & - levels=merge(wp_levels, wp_levels, present(levels)), & - colormap=merge(colormap, "", present(colormap)), & - show_colorbar=merge(show_colorbar, .false., present(show_colorbar)), & - label=merge(label, "", present(label))) + ! Forward parameters to underlying method using conditional calls for memory safety + if (present(levels) .and. present(colormap) .and. present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, show_colorbar=show_colorbar, label=label) + else if (present(levels) .and. present(colormap) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, show_colorbar=show_colorbar) + else if (present(levels) .and. present(colormap) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, & + colormap=colormap, label=label) + else if (present(colormap) .and. present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, & + show_colorbar=show_colorbar, label=label) + else if (present(levels) .and. present(colormap)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, colormap=colormap) + else if (present(levels) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, show_colorbar=show_colorbar) + else if (present(levels) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels, label=label) + else if (present(colormap) .and. present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, show_colorbar=show_colorbar) + else if (present(colormap) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap, label=label) + else if (present(show_colorbar) .and. present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, show_colorbar=show_colorbar, label=label) + else if (present(levels)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, levels=wp_levels) + else if (present(colormap)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, colormap=colormap) + else if (present(show_colorbar)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, show_colorbar=show_colorbar) + else if (present(label)) then + call fig%add_contour_filled(wp_x, wp_y, wp_z, label=label) + else + call fig%add_contour_filled(wp_x, wp_y, wp_z) + end if deallocate(wp_x, wp_y, wp_z) if (allocated(wp_levels)) deallocate(wp_levels) diff --git a/test/test_contour_memory_safety_regression.f90 b/test/test_contour_memory_safety_regression.f90 new file mode 100644 index 00000000..17632f2b --- /dev/null +++ b/test/test_contour_memory_safety_regression.f90 @@ -0,0 +1,113 @@ +program test_contour_memory_safety_regression + !! Test to prevent memory safety regressions in contour plotting + !! Specifically tests the segfault issue fixed in Issue #401 related to merge() function + use fortplot + implicit none + + call test_contour_filled_no_optional_args() + call test_contour_filled_with_colormap_only() + call test_add_contour_filled_no_optional_args() + call test_add_contour_filled_with_colormap_only() + + print *, "All contour memory safety regression tests passed!" + +contains + + subroutine test_contour_filled_no_optional_args() + !! Test contour_filled with no optional arguments - this caused segfault in Issue #401 + real(wp), dimension(10) :: x_grid, y_grid + real(wp), dimension(10,10) :: z_grid + integer :: i, j + + ! Generate simple test data + do i = 1, 10 + x_grid(i) = (i-1) * 0.5_wp + y_grid(i) = (i-1) * 0.5_wp + end do + + do i = 1, 10 + do j = 1, 10 + z_grid(i,j) = sin(x_grid(i)) * cos(y_grid(j)) + end do + end do + + ! This should NOT segfault - the merge() issue was here + call figure() + call contour_filled(x_grid, y_grid, z_grid) + + print *, "✓ contour_filled with no optional args - PASSED" + end subroutine test_contour_filled_no_optional_args + + subroutine test_contour_filled_with_colormap_only() + !! Test contour_filled with only colormap argument + real(wp), dimension(5) :: x_grid, y_grid + real(wp), dimension(5,5) :: z_grid + integer :: i, j + + ! Generate simple test data + do i = 1, 5 + x_grid(i) = (i-1) * 1.0_wp + y_grid(i) = (i-1) * 1.0_wp + end do + + do i = 1, 5 + do j = 1, 5 + z_grid(i,j) = x_grid(i) + y_grid(j) + end do + end do + + call figure() + call contour_filled(x_grid, y_grid, z_grid, colormap="plasma") + + print *, "✓ contour_filled with colormap only - PASSED" + end subroutine test_contour_filled_with_colormap_only + + subroutine test_add_contour_filled_no_optional_args() + !! Test add_contour_filled with no optional arguments + real(wp), dimension(8) :: x_grid, y_grid + real(wp), dimension(8,8) :: z_grid + integer :: i, j + + ! Generate simple test data + do i = 1, 8 + x_grid(i) = (i-1) * 0.25_wp + y_grid(i) = (i-1) * 0.25_wp + end do + + do i = 1, 8 + do j = 1, 8 + z_grid(i,j) = exp(-(x_grid(i)**2 + y_grid(j)**2)) + end do + end do + + call figure() + call add_contour_filled(x_grid, y_grid, z_grid) + + print *, "✓ add_contour_filled with no optional args - PASSED" + end subroutine test_add_contour_filled_no_optional_args + + subroutine test_add_contour_filled_with_colormap_only() + !! Test add_contour_filled with only colormap argument + real(wp), dimension(6) :: x_grid, y_grid + real(wp), dimension(6,6) :: z_grid + integer :: i, j + + ! Generate simple test data + do i = 1, 6 + x_grid(i) = (i-1) * 0.4_wp + y_grid(i) = (i-1) * 0.4_wp + end do + + do i = 1, 6 + do j = 1, 6 + z_grid(i,j) = x_grid(i)**2 - y_grid(j)**2 + end do + end do + + call figure() + call add_contour_filled(x_grid, y_grid, z_grid, colormap="inferno") + + print *, "✓ add_contour_filled with colormap only - PASSED" + end subroutine test_add_contour_filled_with_colormap_only + +end program test_contour_memory_safety_regression \ No newline at end of file From f590e448805432c77398319f7a6376aa3125543b Mon Sep 17 00:00:00 2001 From: Christopher Albert Date: Tue, 26 Aug 2025 17:29:11 +0200 Subject: [PATCH 3/3] plan: add issue #403 to BACKLOG.md TODO after code quality review --- BACKLOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/BACKLOG.md b/BACKLOG.md index a54deb2d..9f5a4c1f 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -12,6 +12,7 @@ **Infrastructure & Documentation Issues (Lower Priority)** - [ ] #388: Fix - investigate test_mpeg_consolidated failure unrelated to ylabel rotation (test infrastructure) +- [ ] #403: Refactor - reduce contour function complexity - functions exceed 50-line target ## DOING (Current Work) - [x] #401: Fix - CI test-coverage failing with segfaults in colored_contours and ascii_heatmap_demo (branch: fix-ci-segfaults-401)