Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Chung <chiahsuan.chung@amd.com>
To: <igt-dev@lists.freedesktop.org>
Cc: Tom Chung <chiahsuan.chung@amd.com>
Subject: [igt-dev] [i-g-t, v2 2/8] tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption
Date: Fri, 30 Jun 2023 19:18:15 +0800	[thread overview]
Message-ID: <20230630111821.126265-3-chiahsuan.chung@amd.com> (raw)
In-Reply-To: <20230630111821.126265-1-chiahsuan.chung@amd.com>

[Why]
There are some memory leak and corruption during the test.

[How]
1. Memory allocated for cicle_sprite.data is too small for drawing the circle.
   Adjust the circle radius from 50 to 48 to fit the cicle_sprite.data size.
2. Pointer preferred_mode does not point to a correct location.
   Use the data->hdisplay and data->vdisplay instead of it.
3. Fix some code for boundary issues.
4. Add some error check for potential memory allocate failed.
5. Free some allocated memory and resources before exit test.
6. Use memcpy() to copy the data->modes data.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 61 +++++++++++++++++---------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index f38d7ce0c..79e3e493c 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -326,8 +326,8 @@ static void sprite_anim_init(void)
 	sprite_init(&cicle_sprite, 100, 100);
 
 	sprite_draw_rect(&cicle_sprite, 0, 0, 100, 100, MK_COLOR(128, 128, 128));
-	/* draw filled circle with center (50, 50), radius 50. */
-	sprite_draw_circle(&cicle_sprite, 50, 50, 50, MK_COLOR(0, 0, 255));
+	/* draw filled circle with center (50, 50), radius 48. */
+	sprite_draw_circle(&cicle_sprite, 50, 50, 48, MK_COLOR(0, 0, 255));
 }
 
 static void sprite_anim(data_t *data, uint32_t *addr)
@@ -414,7 +414,7 @@ static drmModeModeInfo *select_mode(
 				break;
 			}
 		}
-		if (i == data->count_modes)
+		if (i >= data->count_modes)
 			mode = NULL;
 		break;
 
@@ -426,7 +426,7 @@ static drmModeModeInfo *select_mode(
 				break;
 			}
 		}
-		if (i == data->count_modes)
+		if (i >= data->count_modes)
 			mode = NULL;
 		break;
 
@@ -506,7 +506,7 @@ static uint64_t nsec_per_frame(uint64_t refresh)
 static range_t
 get_vrr_range(data_t *data, igt_output_t *output)
 {
-	char buf[256];
+	char buf[256] = {0};
 	char *start_loc;
 	int fd, res;
 	range_t range;
@@ -557,19 +557,25 @@ static void prepare_test(
 
 	/* Prepare resources */
 	if (!data->fb_initialized) {
-		igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-				DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[0]);
+		int fb_id;
+
+		fb_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[0]);
+		igt_assert(fb_id);
+
+		fb_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[1]);
+		igt_assert(fb_id);
 
-		igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-				DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[1]);
 		data->fb_mem[0] = igt_fb_map_buffer(data->drm_fd, &data->fbs[0]);
 		data->fb_mem[1] = igt_fb_map_buffer(data->drm_fd, &data->fbs[1]);
+
+		fbmem_draw_smpte_pattern(data->fb_mem[0], data->hdisplay, data->vdisplay);
+		fbmem_draw_smpte_pattern(data->fb_mem[1], data->hdisplay, data->vdisplay);
+
 		data->fb_initialized = true;
 	}
 
-	fbmem_draw_smpte_pattern(data->fb_mem[0], data->hdisplay, data->vdisplay);
-	fbmem_draw_smpte_pattern(data->fb_mem[1], data->hdisplay, data->vdisplay);
-
 	/* Take care of any required modesetting before the test begins. */
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(data->primary, &data->fbs[0]);
@@ -677,21 +683,21 @@ flip_and_measure(
 static void init_data(data_t *data, igt_output_t *output)
 {
 	int i;
-	uint32_t pm_hdisplay, pm_vdisplay, max_clk = 0;
-	drmModeModeInfo *preferred_mode;
+	uint32_t max_clk = 0;
 	drmModeConnector *connector;
 
 	connector = data->connector = output->config.connector;
 	data->count_modes = connector->count_modes;
 	data->modes = (drmModeModeInfo *)malloc(sizeof(drmModeModeInfo) * data->count_modes);
+	igt_assert(data->modes);
+	memcpy(data->modes, connector->modes, sizeof(drmModeModeInfo) * data->count_modes);
 
-	for (i = 0; i < data->count_modes; i++) {
-		data->modes[i] = connector->modes[i];
 #ifdef FSV_DEBUG
+	for (i = 0; i < data->count_modes; i++) {
 		igt_info("mode %d:", i);
 		kmstest_dump_mode(&data->modes[i]);
-#endif
 	}
+#endif
 
 	/* searching the preferred mode */
 	for (i = 0; i < connector->count_modes; i++) {
@@ -701,8 +707,6 @@ static void init_data(data_t *data, igt_output_t *output)
 			data->preferred_mode_index = i;
 			data->hdisplay = mode->hdisplay;
 			data->vdisplay = mode->vdisplay;
-			pm_hdisplay = preferred_mode->hdisplay;
-			pm_vdisplay = preferred_mode->vdisplay;
 			break;
 		}
 	}
@@ -711,7 +715,7 @@ static void init_data(data_t *data, igt_output_t *output)
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
 
-		if (mode->hdisplay == pm_hdisplay && mode->vdisplay == pm_vdisplay) {
+		if (mode->hdisplay == data->hdisplay && mode->vdisplay == data->vdisplay) {
 			if (mode->clock > max_clk) {
 				max_clk = mode->clock;
 				data->base_mode_index = i;
@@ -742,11 +746,22 @@ static void finish_test(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_fb_unmap_buffer(&data->fbs[0], data->fb_mem[0]);
 	igt_remove_fb(data->drm_fd, &data->fbs[1]);
 	igt_remove_fb(data->drm_fd, &data->fbs[0]);
+
+	data->fb_initialized = false;
+	if (data->modes) {
+		free(data->modes);
+		data->modes = NULL;
+	}
+	if (cicle_sprite.data) {
+		free(cicle_sprite.data);
+		memset(&cicle_sprite, 0, sizeof(cicle_sprite));
+	}
 }
 
 static void
 mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t scene)
 {
+	int ret;
 	uint32_t result;
 	uint64_t interval;
 	drmModeModeInfo *mode_start = NULL, *mode_playback = NULL, mode_custom;
@@ -770,7 +785,8 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 		break;
 	case SCENE_BASE_MODE_TO_CUSTUM_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
-		prepare_custom_mode(data, &mode_custom, 72);
+		ret = prepare_custom_mode(data, &mode_custom, 72);
+		igt_assert_eq(ret, 0);
 		mode_playback = &mode_custom;
 		break;
 	case SCENE_NON_FSV_MODE_TO_NON_FSV_MODE:
@@ -861,5 +877,8 @@ igt_main {
 
 	igt_fixture {
 		igt_display_fini(&data.display);
+		close(data.drm_fd);
+		if (data.modes)
+			free(data.modes);
 	}
 }
-- 
2.25.1

  parent reply	other threads:[~2023-06-30 11:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-30 11:18 [igt-dev] [i-g-t,v2 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing Tom Chung
2023-06-30 11:18 ` Tom Chung [this message]
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 3/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting during the test Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 4/8] tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to resolve tearing issue Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 5/8] tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress messages Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 6/8] tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display reset Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 7/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for some panel Tom Chung
2023-06-30 11:18 ` [igt-dev] [i-g-t, v2 8/8] tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to meson.build Tom Chung
2023-06-30 16:31   ` Kamil Konieczny
2023-06-30 13:27 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Fix some bugs in amd_freesync_video_mode Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230630111821.126265-3-chiahsuan.chung@amd.com \
    --to=chiahsuan.chung@amd.com \
    --cc=igt-dev@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox