[PATCH] kernel32/tests: Allow failure when loading the 268 bytes minimal PE image.
Contrary to the comment Windows 7+ fails to load it (but maybe unpatched pre-SP1 Windows 7 versions could load it). So adjust the comment and allow ERROR_BAD_EXE_FORMAT as a valid result. Wine-Bug: https://bugs.winehq.org//show_bug.cgi?id=51185 Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- Another option would be to remove this case entirely... https://test.winehq.org/data/patterns.html#kernel32:loader --- dlls/kernel32/tests/loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 1db68a191a3..ea4048d5634 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -860,15 +860,15 @@ static void test_Loader(void) 0, { ERROR_SUCCESS, ERROR_BAD_EXE_FORMAT } /* vista is more strict */ }, - /* Minimal PE image that Windows7 is able to load: 268 bytes */ + /* Minimal PE image that Windows Vista is able to load: 268 bytes */ { 0x04, 0, 0xf0, /* optional header size just forces 0xf0 bytes to be written, 0 or another number don't change the behaviour, what really matters is file size regardless of values in the headers */ 0x04 /* also serves as e_lfanew in the truncated MZ header */, 0x04, - 0x40, /* minimal image size that Windows7 accepts */ + 0x40, /* minimal image size that Vista accepts */ 0, - { ERROR_SUCCESS } + { ERROR_SUCCESS, ERROR_BAD_EXE_FORMAT } /* most win 7+ are more strict */ }, /* the following data mimics the PE image which 8k demos have */ { 0x04, -- 2.20.1
Francois Gouget <fgouget(a)codeweavers.com> wrote:
- /* Minimal PE image that Windows7 is able to load: 268 bytes */ + /* Minimal PE image that Windows Vista is able to load: 268 bytes */ { 0x04, 0, 0xf0, /* optional header size just forces 0xf0 bytes to be written, 0 or another number don't change the behaviour, what really matters is file size regardless of values in the headers */ 0x04 /* also serves as e_lfanew in the truncated MZ header */, 0x04, - 0x40, /* minimal image size that Windows7 accepts */ + 0x40, /* minimal image size that Vista accepts */ 0, - { ERROR_SUCCESS } + { ERROR_SUCCESS, ERROR_BAD_EXE_FORMAT } /* most win 7+ are more strict */
I have created this test under Windows 7, so at that time this definitely worked. While I'm not opposed to add ERROR_BAD_EXE_FORMAT as a fallback error value, I do oppose to changing in the comment Windows7 to Vista, Vista is an utterly broken platform in many aspects, promoting its name to a not related test result is not fair. -- Dmitry.
On Fri, 16 Jul 2021, Dmitry Timoshkov wrote: [...]
I have created this test under Windows 7, so at that time this definitely worked. While I'm not opposed to add ERROR_BAD_EXE_FORMAT as a fallback error value, I do oppose to changing in the comment Windows7 to Vista, Vista is an utterly broken platform in many aspects, promoting its name to a not related test result is not fair.
WTH??? Where do you see Vista being promoted? What the heck is this thing about fairness? What do your likes and dislikes have to do with the accuracy of the comment? What you could have said is that it's not just the Vista machines that are missing from the failure pattern but also w7pro64 which means that either the test succeeds or is skipped there. https://test.winehq.org/data/patterns.html#kernel32:loader Another constructive contribution would have been to point to the WineTest results that show that the 64-bit test is being run with no failure on w7pro64 and no skip message. Then point out that there is still some doubt due to the break on line 1123. https://test.winehq.org/data/2ffcf4402181e7e7e8c322b713924e5738506c88/win7_n... So the next step would have been to manually submit a job with a couple of extra traces providing direct proof that this test succeeds in a 32-bit kernel32_test.exe process on w7pro64: https://testbot.winehq.org/JobDetails.pl?Key=94182&f205=exe32.report#k205 loader.c:918: 16: LoadLibraryA succeeded That would have proven that yes, this test does succeed on Windows 7, at least some in some cases. Leaving us with one question: why does it succeed sometimes and not others? Now you're lucky. I still decided to do some extra tests: * I have run the tests on various snapshots of the w7u, w7pro64 VMs and my win7u-64 VMware machine. They all show that the service packs do not change the result. * Looking at just the Windows 7 results, the only one that has the failure is the 32-bit VM. Coincidence? * But looking at the Windows 8.1 machines we see that it fails on cw-rx460 but not on cw-gtx560. Yet they are both 64-bit and have the same Windows 8.1 version... but not the exact same kernel32 version. Still, no clear pattern appears when adding w8 and w864 to the mix: w8 6.3.9600.17056 success w864 6.3.9600.17056 success cw-rx460 6.3.9600.17415 failure cw-gtx560 6.3.9600.19994 success Yet another Radeon mystery :-( This also tends to invalidate the above '32-bit OS' theory. * Then I grepped all the WineTest reports which confirmed that the test fails systematically on all Windows 10 >= 1809. This is also confirmed by this TestBot job. https://testbot.winehq.org/JobDetails.pl?Key=94183 So to summarize: * Vista : 100% success * Windows 7 : mixed results * Windows 8.1 : mixed results * Windows 10 1507-1709 : 100% success * Windows 10 1809+ : 100% failure In other words: * We don't really know what determines whether the test succeeds or fails. * "minimal image size that Windows7 accepts" is incorrect. "minimal image size that some Windows7 accept" would be more correct. "minimal image size that Vista accepts" is correct based on our limited Vista results. But it's also imprecise because the test also succeeds on many post-Vista Windows versions. * Could this have something to do with Microsoft Defender? (which cannot be disabled in recent Windows 10 versions and may not like the structure of this test dll) I'll resubmit this patch with a reworded comment to reflect the current knowledge. -- François Gouget <fgouget(a)free.fr>
On Sat, 17 Jul 2021, François Gouget wrote: [...]
* Could this have something to do with Microsoft Defender? (which cannot be disabled in recent Windows 10 versions and may not like the structure of this test dll)
I used Microsoft Defender's temporary suspension feature to check that theory and it did not make any difference. So that's not it. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Linux: It is now safe to turn on your computer.
François Gouget <fgouget(a)free.fr> wrote:
On Fri, 16 Jul 2021, Dmitry Timoshkov wrote: [...]
I have created this test under Windows 7, so at that time this definitely worked. While I'm not opposed to add ERROR_BAD_EXE_FORMAT as a fallback error value, I do oppose to changing in the comment Windows7 to Vista, Vista is an utterly broken platform in many aspects, promoting its name to a not related test result is not fair.
WTH??? Where do you see Vista being promoted? What the heck is this thing about fairness? What do your likes and dislikes have to do with the accuracy of the comment?
What I meant by using the word "promoting" is simply the fact of changing original meaning that the test represents Windows 7 behaviour, and that using Vista in that context is not appropriate. I addition, at the time the test was submitted and accepted, it did successfully pass under the testbot VMs. Since you are the testbot maintainer, and were maintaining testbot also in the time frame when this test was committed, I'd guess it's the question to you, what changed since then that the test no longer passes. P.S. If you can't for some reason stay in bounds, please refrain from sending offensive posts, of course if your original intent is not to be ignored. -- Dmitry.
On Sun, 18 Jul 2021, Dmitry Timoshkov wrote: [...]
P.S. If you can't for some reason stay in bounds, please refrain from sending offensive posts, of course if your original intent is not to be ignored.
Now that's a good one. You are the one sending me two offensive emails in a row. Yes I mean this email: | I do oppose to changing in the comment Windows7 to Vista, Vista is | an utterly broken platform in many aspects, promoting its name to | a not related test result is not fair. Following this one by the way: | Hiding ok() inside of an if() is one of the worst things I've ever | seen. But somehow I am the one being offensive. Do you even realize how routinely offensive and even insulting your emails are? Do you realize I spend a considerable time humoring you and trying to get to the bottom of this issue? Either your attitude changes or that will stop right there. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Chemistry professors never die, they just fail to react.
participants (4)
-
Dmitry Timoshkov -
Francois Gouget -
Francois Gouget -
François Gouget