[PATCH 0/2] MR8297: msvcirt/tests: Fix 2 memory issues in tests
From: Yuxuan Shui <yshui(a)codeweavers.com> Instead of logging value from the current buffer, we used a saved buffer pointer which has already been freed. --- dlls/msvcirt/tests/msvcirt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/msvcirt/tests/msvcirt.c b/dlls/msvcirt/tests/msvcirt.c index a22984a3e86..8b20c6095e3 100644 --- a/dlls/msvcirt/tests/msvcirt.c +++ b/dlls/msvcirt/tests/msvcirt.c @@ -7380,7 +7380,7 @@ static void test_ifstream(void) call_func5(p_ifstream_buffer_ctor, &ifs, -1, NULL, 0, TRUE); ok(ifs.base_ios.sb->base == NULL, "wrong base value, expected NULL got %p\n", ifs.base_ios.sb->base); ok(ifs.base_ios.sb->ebuf == NULL, "wrong ebuf value, expected NULL got %p\n", ifs.base_ios.sb->ebuf); - ok(ifs.base_ios.sb->unbuffered == 1, "wrong unbuffered value, expected 1 got %d\n", pfb->base.unbuffered); + ok(ifs.base_ios.sb->unbuffered == 1, "wrong unbuffered value, expected 1 got %d\n", ifs.base_ios.sb->unbuffered); ok(ifs.base_ios.sb->allocated == 0, "wrong allocated value, expected 0 got %d\n", ifs.base_ios.sb->allocated); psb = call_func3(p_ifstream_setbuf, &ifs, buffer, ARRAY_SIZE(buffer)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8297
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/msvcirt/tests/msvcirt.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/msvcirt/tests/msvcirt.c b/dlls/msvcirt/tests/msvcirt.c index 8b20c6095e3..bf2b6381608 100644 --- a/dlls/msvcirt/tests/msvcirt.c +++ b/dlls/msvcirt/tests/msvcirt.c @@ -2265,6 +2265,15 @@ static void test_filebuf(void) CloseHandle(thread); } +static void *custom_alloc(LONG size) +{ + /* this allocates size + 16 bytes. a test later validates that strstreambuf_overflow + * always allocates in `strstreambuf.increase` increments, even when the resultant buffer + * won't be large enough to contain `pptr`. to avoid actually writing out-of-bound, we + * allocate a few bytes more here. */ + return p_operator_new(size + 16); +} + static void test_strstreambuf(void) { strstreambuf ssb1, ssb2; @@ -2532,8 +2541,12 @@ static void test_strstreambuf(void) ok(ssb2.base.pptr == ssb2.base.base + 1, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 1, ssb2.base.pptr); ok(ssb2.base.epptr == ssb2.base.base + 16, "wrong put end, expected %p got %p\n", ssb2.base.base + 16, ssb2.base.epptr); ok(*(ssb2.base.pptr - 1) == 'D', "expected 'D' got %d\n", *(ssb2.base.pptr - 1)); + /* test strstreambuf_overflow always allocate in `increase` increments, even when the new size + * won't be large enough to contain `pptr`. `custom_alloc` allocate more space than asked for + * to avoid out-of-bound access. */ ssb2.base.pbase = ssb2.base.base + 3; ssb2.base.pptr = ssb2.base.epptr + 5; + ssb2.f_alloc = custom_alloc; ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'E'); ok(ret == 1, "expected 1 got %d\n", ret); ok(ssb2.base.ebuf == ssb2.base.base + 20, "expected %p got %p\n", ssb2.base.base + 20, ssb2.base.ebuf); @@ -2542,6 +2555,7 @@ static void test_strstreambuf(void) ok(ssb2.base.pptr == ssb2.base.base + 22, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 22, ssb2.base.pptr); ok(ssb2.base.epptr == ssb2.base.base + 20, "wrong put end, expected %p got %p\n", ssb2.base.base + 20, ssb2.base.epptr); ok(*(ssb2.base.pptr - 1) == 'E', "expected 'E' got %d\n", *(ssb2.base.pptr - 1)); + ssb2.f_alloc = NULL; ssb2.base.egptr = ssb2.base.base + 2; ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'F'); ok(ret == 1, "expected 1 got %d\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8297
Piotr Caban (@piotr) commented about dlls/msvcirt/tests/msvcirt.c:
ok(ssb2.base.pptr == ssb2.base.base + 1, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 1, ssb2.base.pptr); ok(ssb2.base.epptr == ssb2.base.base + 16, "wrong put end, expected %p got %p\n", ssb2.base.base + 16, ssb2.base.epptr); ok(*(ssb2.base.pptr - 1) == 'D', "expected 'D' got %d\n", *(ssb2.base.pptr - 1)); + /* test strstreambuf_overflow always allocate in `increase` increments, even when the new size + * won't be large enough to contain `pptr`. `custom_alloc` allocate more space than asked for + * to avoid out-of-bound access. */ ssb2.base.pbase = ssb2.base.base + 3; ssb2.base.pptr = ssb2.base.epptr + 5; + ssb2.f_alloc = custom_alloc; ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'E');
Can we change the test to call strstreambuf_doallocate instead? I think it would be cleaner this way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8297#note_106499
On Fri Jun 13 15:35:53 2025 +0000, Piotr Caban wrote:
Can we change the test to call strstreambuf_doallocate instead? I think it would be cleaner this way. Did you mean call `strstreambuf_doallocate` directly instead of `strstreambuf_overflow`? We can do that but I feel if we do that we can't rule out the possibility that `strstreambuf_overflow` may call `doallocate` multiple times. (i.e. `doallocate` does one `increase` increment, but `overflow` does multiple).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8297#note_106558
On Sat Jun 14 01:35:28 2025 +0000, Yuxuan Shui wrote:
Did you mean call `strstreambuf_doallocate` directly instead of `strstreambuf_overflow`? We can do that but I feel if we do that we can't rule out the possibility that `strstreambuf_overflow` may call `doallocate` multiple times. (i.e. `doallocate` does one `increase` increment, but `overflow` does multiple). I was not sure if there's value in testing that.It's possible to set the pointers using exported functions so the test shows limitation of native implementation. I think I'm good with current approach.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8297#note_106574
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8297
participants (3)
-
Piotr Caban (＠piotr) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)