ntdll: Increment offset by len in build_clr_surrogate_section.
Instead of multiplying it by 2. I don't know how this ever worked.
Merge request reports
Activity
requested review from @nsivov
assigned to @alexhenrie
- Resolved by Alex Henrie
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=131843 Your paranoid android. === w7pro64 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w864 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w1064v1507 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w1064v1809 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w1064_2qxl (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w1064_adm (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w1064_tsign (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w10pro64 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w10pro64_en_AE_u8 (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w10pro64_ar (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w10pro64_ja (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w10pro64_zh_CN (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105 === w11pro64_amd (64 bit report) === kernel32: actctx.c:2823: Test failed: got error 14105
1871 1878 ok_(__FILE__, line)(data.hActCtx == NULL, "data.hActCtx=%p\n", data.hActCtx); 1872 1879 ok_(__FILE__, line)(data.ulAssemblyRosterIndex == exid, "data.ulAssemblyRosterIndex=%lu, expected %lu\n", 1873 1880 data.ulAssemblyRosterIndex, exid); 1881 1882 header = (struct guidsection_header*)data.lpSectionBase; 1883 ok(header->magic == 0x64487347, "got wrong magic 0x%08lx\n", header->magic); 1884 ok(header->size == sizeof(*header), "got size %ld\n", header->size); 1885 ok(header->count == count, "got count %ld, expected %ld\n", header->count, count); 1886 ok(header->index_offset == sizeof(*header), "got index offset %lu\n", header->index_offset); 1887 1888 index = (struct guid_index*)((BYTE*)data.lpSectionBase + header->index_offset); 1889 ok(index[0].data_offset == sizeof(*header) + count * sizeof(*index), "got data offset %ld\n", index[0].data_offset); 1890 ok(index[0].rosterindex == exid, "got roster index %ld, expected %lu\n", index[0].rosterindex, exid); 1891 ok(index[1].data_offset == index[0].data_offset + index[0].data_len, "got data offset %lu, expected %lu\n", 1892 index[1].data_offset, index[0].data_offset + index[0].data_len); 1893 ok(index[1].rosterindex == exid, "got roster index %ld, expected %lu\n", index[1].rosterindex, exid); - Comment on lines +1881 to +1893
I was thinking of having some helper that would validate guid index, and another one for string index (unrelated to your fix). There is no need to pass in number of entries, it's fine to use what's in the header. Then we'd call for index validation for all sections, it's possible there are more issues like what you've found.
Today I tried writing generic tests to check any section header for consistency. The problem I ran into is that Windows is not consistent about where it puts blocks of data in the sections. Sometimes the blocks are not contiguous and sometimes they are out of order compared to the index. It would seem that the exact layout is unimportant.
My preference would be to fix the obviously broken code now and worry about adding tests later. If the one-line fix cannot be accepted into mainline Wine without more tests, let's put it in Wine Staging until better tests are written.
It's not obvious, because tests are working for me without the fix somehow - if you add another clr entry, it is found correctly. I don't see why you'd mention wine-staging for this case if the fix is obvious to you, line count is not a metric to use.
I'll take a look at additional tests that I suggested.
Please take a look. Testing that offset is within section bounds is enough to show what your fix changes. Note that your test is broken, because you can't have duplicate GUIDs in the same section. I'm not sure if we enforce that yet.
changed this line in version 3 of the diff
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132041 Your paranoid android. === w7pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w864 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1507 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1809 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_2qxl (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_adm (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_tsign (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_en_AE_u8 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ar (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ja (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_zh_CN (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w11pro64_amd (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105
added 258 commits
-
31106f6f...cd9f3e5d - 256 commits from branch
wine:master
- c400db7d - kernel32/tests: Test internal consistency of activation context headers.
- 0e129f76 - ntdll: Increment offset by len in build_clr_surrogate_section.
-
31106f6f...cd9f3e5d - 256 commits from branch
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132042 Your paranoid android. === w7pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w864 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1507 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064v1809 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_2qxl (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_adm (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w1064_tsign (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_en_AE_u8 (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ar (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_ja (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w10pro64_zh_CN (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105 === w11pro64_amd (64 bit report) === kernel32: actctx.c:2874: Test failed: got error 14105
That's what I did in !2618 (comment 30547).
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132073 Your paranoid android. === w7pro64 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w864 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w1064v1507 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w1064v1809 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w1064_2qxl (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w1064_adm (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w1064_tsign (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w10pro64 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w10pro64_en_AE_u8 (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w10pro64_ar (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w10pro64_ja (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w10pro64_zh_CN (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105 === w11pro64_amd (64 bit report) === kernel32: actctx.c:2839: Test failed: got error 14105
added 38 commits
-
ef28221f...2371906a - 36 commits from branch
wine:master
- bd745957 - kernel32/tests: Check that lpSectionBase != NULL before dereferencing.
- 7af7ff87 - ntdll: Increment offset by len in build_clr_surrogate_section.
-
ef28221f...2371906a - 36 commits from branch