Skip to content

Conversation

@pushkarscripts
Copy link

@pushkarscripts pushkarscripts commented Jan 11, 2026

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository.
  • I made sure to follow the project's coding style.
  • I've documented every RZ_API function and struct this PR changes.
  • I've added or updated tests that prove my changes are effective.
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

LeakSanitizer was reporting leaks in the mark unit tests when running
test_rz_mark_get_all_off() and test_rz_mark_all_list().

Both functions return owned RzList objects, but the tests were not freeing them, which caused the leaks. This change updates the tests to release those lists properly so ASan runs are clean.

Also fixes a memory leak in test_analysis_il where the value returned by rz_sys_getenv() was not freed.

Test plan

Built and tested using AddressSanitizer:

meson setup build-asan -Db_sanitize=address,undefined
ninja -C build-asan
ASAN_OPTIONS=abort_on_error=1:print_stacktrace=1
UBSAN_OPTIONS=abort_on_error=1:print_stacktrace=1
./build-asan/test/unit/test_marks

Output after the fix:

test_rz_mark_get_at OK
test_rz_mark_get_all_off OK
test_rz_mark_all_list OK
test_rz_mark_unset OK
test_rz_mark_set_properties OK
test_rz_mark_rename OK
test_rz_mark_count_and_starts_or_ends OK

No ASan/LSan errors reported.

test/integration/test_analysis_il also passes cleanly under ASan.

Closing issues

@pushkarscripts
Copy link
Author

Thanks for pointing that out.
You are right, that change was unnecessary. I have reverted the mark.c part and kept only the test fixes.

Copy link
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests don't pass

Comment on lines +56 to +62
char *tb = rz_sys_getenv("RZ_TESTBINS");
mu_assert_notnull(tb, "RZ_TESTBINS not set");

char *path = rz_str_newf("%s/elf/emulateme.arm64", tb);
rz_mem_free(tb);
RzCoreFile *cf = rz_core_file_open(core, path, RZ_PERM_RWX, 0);
rz_mem_free(path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you run this code? seems AI generated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I ran this locally under ASan. You're right about rz_sys_getenv, I treated it as owned, which was wrong.

I'll fix the ownership and update the PR.

@Rot127 Rot127 added the Requirements not met The PR doesn't meet the minimum contribution requirements. See CONTRIBUTING.md for details. label Jan 13, 2026
@Rot127 Rot127 marked this pull request as draft January 13, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Requirements not met The PR doesn't meet the minimum contribution requirements. See CONTRIBUTING.md for details. rz-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants