June 2018 - Wine-Devel - list.winehq.org

[PATCH 2/2] testbot/build: Add proper command line options to Build.pl and Reconfig.pl.

by Francois Gouget

This makes it much easier to test the scripts directly on the VM. This also makes it possible to test one aspect of the scripts without having to go through the whole process. In particular the --no-rm Reconfig.pl option speeds up testing by avoiding rebuilding from scratch. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/build/Build.pl | 175 +++++++++++++++++++++++----------- testbot/bin/build/Reconfig.pl | 137 +++++++++++++++++++++++--- 2 files changed, 241 insertions(+), 71 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 17ae74ff5..4df107005 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -43,12 +43,19 @@ sub BEGIN } $::BuildEnv = 1; } +my $Name0 = $0; +$Name0 =~ s+^.*/++; + use WineTestBot::Config; use WineTestBot::PatchUtils; use WineTestBot::Utils; +# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -59,12 +66,16 @@ sub LogMsg(@) print "Build: ", @_; } -sub FatalError(@) +sub Error(@) { - print STDERR @_; - exit 1; + print STDERR "$Name0:error: ", @_; } + +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -133,9 +144,11 @@ sub BuildNative() return 1; } -sub BuildTestExecutables($$) +sub BuildTestExecutables($$$) { - my ($Impacts, $Bits) = @_; + my ($Targets, $Impacts, $Bits) = @_; + + return 1 if (!$Targets->{"exe$Bits"}); my (@BuildDirs, @TestExes); foreach my $TestInfo (values %{$Impacts->{Tests}}) @@ -168,62 +181,113 @@ sub BuildTestExecutables($$) return $Success; } + +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; -my ($PatchFile, $BitIndicators); -if (@ARGV == 2) -{ - ($PatchFile, $BitIndicators) = @ARGV; -} -else -{ - # FIXME Remove support for the legacy parameters - my ($_PatchType, $_BaseName); - ($PatchFile, $_PatchType, $_BaseName, $BitIndicators) = @ARGV; -} -if (! $PatchFile || !$BitIndicators) -{ - FatalError "Usage: Build.pl <patchfile> <bits>\n"; -} - -# Verify parameters -if (!IsValidFileName($PatchFile)) -{ - FatalError "The patch filename '$PatchFile' contains invalid characters\n"; -} -$PatchFile = "$DataDir/staging/$PatchFile"; -if (!-r $PatchFile) -{ - FatalError "Patch file '$PatchFile' is not readable\n"; -} +my %AllTargets; +map { $AllTargets{$_} = 1 } qw(exe32 exe64); -my ($Run32, $Run64); -if ($BitIndicators =~ m/^([\d,]+)$/) +my ($Usage, $PatchFile, $TargetList); +my $IgnoreNext = 0; # FIXME Backward compatibility +while (@ARGV) { - foreach my $BitsValue (split /,/, $1) + my $Arg = shift @ARGV; + if ($Arg =~ /^patch(?:dlls|programs)$/) { - if ($BitsValue eq "32") - { - $Run32 = 1; - } - elsif ($BitsValue eq "64") + $IgnoreNext ||= 1; # Ignore this legacy parameter + } + elsif ($IgnoreNext == 1) + { + $IgnoreNext = 2; # Ignore this legacy parameter + } + elsif ($Arg =~ /^(?:-\?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $PatchFile) + { + if (IsValidFileName($Arg)) { - $Run64 = 1; + $PatchFile = "$DataDir/staging/$Arg"; + if (!-r $PatchFile) + { + Error "patch file '$Arg' is not readable\n"; + $Usage = 2; + } } else { - FatalError "Invalid number of bits $BitsValue\n"; + Error "the patch filename '$Arg' contains invalid characters\n"; + $Usage = 2; + last; } } - if (!$Run32 && !$Run64) + elsif (!defined $TargetList) { - FatalError "Specify at least one of 32 or 64 bits\n"; + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + if (!defined $PatchFile) + { + Error "you must specify a patch to apply\n"; + $Usage = 2; + } + + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + $Target = "exe$1" if ($Target =~ /^(32|64)$/); + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; } } -else +if (defined $Usage) { - FatalError "Invalid number of bits $BitIndicators\n"; + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--help] PATCHFILE TARGETS\n"; + print "\n"; + print "Applies the specified patch and rebuilds the Wine test executables.\n"; + print "\n"; + print "Where:\n"; + print " PATCHFILE Is the staging file containing the patch to build.\n"; + print " TARGETS Is a comma-separated list of build targets. By default every\n"; + print " target is run.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --help Shows this usage message.\n"; + exit 0; } if ($DataDir =~ /'/) @@ -232,20 +296,19 @@ if ($DataDir =~ /'/) exit(1); } -my $Impacts = ApplyPatch($PatchFile); -exit(1) if (!$Impacts); + +# +# Run the builds +# CountCPUs(); -if ($Impacts->{WineBuild} and !BuildNative()) -{ - exit(1); -} -if ($Run32 && !BuildTestExecutables($Impacts, 32)) -{ - exit(1); -} -if ($Run64 && !BuildTestExecutables($Impacts, 64)) +my $Impacts = ApplyPatch($PatchFile); + +if (!$Impacts or + ($Impacts->{WineBuild} and !BuildNative()) or + !BuildTestExecutables($Targets, $Impacts, 32) or + !BuildTestExecutables($Targets, $Impacts, 64)) { exit(1); } diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 5ecbc2af3..450de0e83 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -38,12 +38,19 @@ sub BEGIN unshift @INC, "$::RootDir/lib"; } $::BuildEnv = 1; - } +} +my $Name0 = $0; +$Name0 =~ s+^.*/++; + use WineTestBot::Config; use WineTestBot::PatchUtils; +# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -54,6 +61,16 @@ sub LogMsg(@) print "Reconfig: ", @_; } +sub Error(@) +{ + print STDERR "$Name0:error: ", @_; +} + + +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -108,8 +125,11 @@ sub BuildTestLauncher() return 1; } -sub GitPull() +sub GitPull($) { + my ($Targets) = @_; + return 1 if (!$Targets->{update}); + InfoMsg "\nUpdating the Wine source\n"; system("cd '$DataDir/wine' && git pull"); if ($? != 0) @@ -128,14 +148,17 @@ sub GitPull() return 1; } -sub BuildNative() +sub BuildNative($$) { + my ($Targets, $NoRm) = @_; + + return 1 if (!$Targets->{native}); mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native"); # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; system("cd '$DataDir/build-native' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "rm -rf * && ") . "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus __tooldeps__"); @@ -148,17 +171,18 @@ sub BuildNative() return 1; } -sub BuildCross($) +sub BuildCross($$$) { - my ($Bits) = @_; + my ($Targets, $NoRm, $Bits) = @_; - my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); - mkdir "$DataDir/build-mingw$Bits" if (! -d "$DataDir/build-mingw$Bits"); + return 1 if (!$Targets->{"exe$Bits"}); + mkdir "$DataDir/build-mingw$Bits" if (!-d "$DataDir/build-mingw$Bits"); # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; + my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); system("cd '$DataDir/build-mingw$Bits' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "rm -rf * && ") . "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus buildtests"); if ($? != 0) @@ -170,9 +194,87 @@ sub BuildCross($) return 1; } + +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; +my %AllTargets; +map { $AllTargets{$_} = 1 } qw(update native exe32 exe64); + +my ($Usage, $TargetList, $NoRm); +while (@ARGV) +{ + my $Arg = shift @ARGV; + if ($Arg eq "--no-rm") + { + $NoRm = 1; + } + elsif ($Arg =~ /^(?:-\?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $TargetList) + { + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; + } +} +if (defined $Usage) +{ + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--no-rm] [--help] [TARGETS]\n"; + print "\n"; + print "Updates Wine to the latest version and recompiles it so the host is ready to build executables for the Windows tests.\n"; + print "\n"; + print "Where:\n"; + print " TARGETS Is a comma-separated list of reconfiguration targets. By default\n"; + print " every target is run.\n"; + print " - update: Update Wine's source code.\n"; + print " - native: Rebuild the native Wine tools.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --no-rm Don't rebuild from scratch.\n"; + print " --help Shows this usage message.\n"; + exit 0; +} + if ($DataDir =~ /'/) { LogMsg "The install path contains invalid characters\n"; @@ -184,14 +286,19 @@ if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") exit(1); } + +# +# Run the builds +# + CountCPUs(); -if (!BuildTestAgentd() || - !BuildTestLauncher() || - !GitPull() || - !BuildNative() || - !BuildCross(32) || - !BuildCross(64)) +if (!BuildTestAgentd() or + !BuildTestLauncher() or + !GitPull($Targets) or + !BuildNative($Targets, $NoRm) or + !BuildCross($Targets, $NoRm, 32) or + !BuildCross($Targets, $NoRm, 64)) { exit(1); } -- 2.17.1

June 21, 2018

[PATCH 1/2] testbot/build: Quote the paths in the shell commands.

by Francois Gouget

Also check that $DataDir does not contain single quotes, thus ensuring our simple quoting scheme works. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- A more absolutist approach would be to use ShQuote() everywhere which would remove the single quote restriction. But that would compromise readability. So this solution seems like a good compromise. Also this is mostly cosmetic in the first place since the administrator would presumably not put spaces, stars, dollar signs or single quotes in the $DataDir path. Still I much prefer having proper quoting in my shell scripts. testbot/bin/build/Build.pl | 16 +++++++++++----- testbot/bin/build/Reconfig.pl | 17 +++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index c23bb8780..17ae74ff5 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -82,7 +82,7 @@ sub ApplyPatch($) my ($PatchFile) = @_; InfoMsg "Applying patch\n"; - system("cd $DataDir/wine && set -x && ". + system("cd '$DataDir/wine' && set -x && ". "git apply --verbose ". ShQuote($PatchFile) ." && ". "git add -A"); if ($? != 0) @@ -95,7 +95,7 @@ sub ApplyPatch($) if ($Impacts->{Makefiles}) { InfoMsg "\nRunning make_makefiles\n"; - system("cd $DataDir/wine && set -x && ./tools/make_makefiles"); + system("cd '$DataDir/wine' && set -x && ./tools/make_makefiles"); if ($? != 0) { LogMsg "make_makefiles failed\n"; @@ -106,7 +106,7 @@ sub ApplyPatch($) if ($Impacts->{Autoconf} && !$Impacts->{HasConfigure}) { InfoMsg "\nRunning autoconf\n"; - system("cd $DataDir/wine && set -x && autoconf"); + system("cd '$DataDir/wine' && set -x && autoconf"); if ($? != 0) { LogMsg "Autoconf failed\n"; @@ -122,7 +122,7 @@ sub BuildNative() mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native"); InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + system("cd '$DataDir/build-native' && set -x && ". "time make -j$ncpus __tooldeps__"); if ($? != 0) { @@ -147,7 +147,7 @@ sub BuildTestExecutables($$) } InfoMsg "\nBuilding the $Bits-bit test executable(s)\n"; - system("cd $DataDir/build-mingw$Bits && set -x && ". + system("cd '$DataDir/build-mingw$Bits' && set -x && ". "time make -j$ncpus ". join(" ", sort @BuildDirs)); if ($? != 0) { @@ -226,6 +226,12 @@ else FatalError "Invalid number of bits $BitIndicators\n"; } +if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} + my $Impacts = ApplyPatch($PatchFile); exit(1) if (!$Impacts); diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 2da2f54a4..5ecbc2af3 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -73,7 +73,7 @@ sub BuildTestAgentd() if (! -x "$BinDir/build/testagentd") { InfoMsg "\nBuilding the native testagentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus build"); if ($? != 0) { @@ -83,7 +83,7 @@ sub BuildTestAgentd() } InfoMsg "\nRebuilding the Windows TestAgentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus iso"); if ($? != 0) { @@ -97,7 +97,7 @@ sub BuildTestAgentd() sub BuildTestLauncher() { InfoMsg "\nRebuilding TestLauncher\n"; - system("cd $::RootDir/src/TestLauncher && set -x && ". + system("cd '$::RootDir/src/TestLauncher' && set -x && ". "time make -j$ncpus"); if ($? != 0) { @@ -111,7 +111,7 @@ sub BuildTestLauncher() sub GitPull() { InfoMsg "\nUpdating the Wine source\n"; - system("cd $DataDir/wine && git pull"); + system("cd '$DataDir/wine' && git pull"); if ($? != 0) { LogMsg "Git pull failed\n"; @@ -134,7 +134,7 @@ sub BuildNative() # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + system("cd '$DataDir/build-native' && set -x && ". "rm -rf * && ". "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus __tooldeps__"); @@ -157,7 +157,7 @@ sub BuildCross($) # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; - system("cd $DataDir/build-mingw$Bits && set -x && ". + system("cd '$DataDir/build-mingw$Bits' && set -x && ". "rm -rf * && ". "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus buildtests"); @@ -173,6 +173,11 @@ sub BuildCross($) $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; +if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") { LogMsg "Unable to create '$DataDir/staging': $!\n"; -- 2.17.1

June 21, 2018

[PATCH 3/3] testbot/WineRunTask: Rename the Windows test reports.

by Francois Gouget

Keep 'log' for the task log messages (such as the compilation log) and store the actual (WineTest) test report in '.report' files. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- A coming patch will put the output of the batch file in the log file (after I have checked that the TestBot VMs have the version of TestAgentd where redirections work correctly). testbot/bin/WineRunTask.pl | 16 ++++++++-------- testbot/web/JobDetails.pl | 9 ++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 7f92b8858..3f5a3ee8d 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -195,7 +195,7 @@ my $OldUMask = umask(002); my $TaskDir = $Task->CreateDir(); umask($OldUMask); my $VM = $Task->VM; -my $RptFileName = $VM->Name . ".rpt"; +my $RptFileName = $Step->FileType .".report"; my $Start = Time(); @@ -292,13 +292,13 @@ sub WrapUpAndExit($;$$$) if ($Step->Type eq 'suite' and $Status eq 'completed' and !$TimedOut) { # Keep the old report if the new one is missing - if (-f "$TaskDir/log" and !-z "$TaskDir/log") + if (-f "$TaskDir/$RptFileName" and !-z "$TaskDir/$RptFileName") { # Update the reference VM suite results for WineSendLog.pl my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", $Step->FileType eq "exe64" ? "64" : "32"); unlink("$LatestBaseName.log"); - link("$TaskDir/log", "$LatestBaseName.log"); + link("$TaskDir/$RptFileName", "$LatestBaseName.log"); unlink("$LatestBaseName.err"); if (-f "$TaskDir/err" and !-z "$TaskDir/err") { @@ -518,11 +518,11 @@ if (!defined $TA->Wait($Pid, $Timeout, $Keepalive)) } my $TimedOut; -Debug(Elapsed($Start), " Retrieving the report file to 'log'\n"); -if ($TA->GetFile($RptFileName, "$TaskDir/log")) +Debug(Elapsed($Start), " Retrieving the report file to '$RptFileName'\n"); +if ($TA->GetFile($RptFileName, "$TaskDir/$RptFileName")) { - chmod 0664, "$TaskDir/log"; - if (open(my $LogFile, "<", "$TaskDir/log")) + chmod 0664, "$TaskDir/$RptFileName"; + if (open(my $LogFile, "<", "$TaskDir/$RptFileName")) { # There is more than one test unit when running the full test suite so keep # track of the current one. Note that for the TestBot we don't count or @@ -788,7 +788,7 @@ if ($TA->GetFile($RptFileName, "$TaskDir/log")) else { $NewStatus = 'boterror'; - Error "Unable to open 'log' for reading: $!\n"; + Error "Unable to open '$RptFileName' for reading: $!\n"; LogTaskError("Unable to open the log file for reading: $!\n"); } } diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 3add52b4c..a1a846ff9 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -280,9 +280,12 @@ sub GetHtmlLine($$$) return (undef, $Line); } +my @MILogFiles = qw(exe32.report exe64.report log log.old); my %MILogLabels = ( - "log" => "task log", - "log.old" => "old logs", + "exe32.report" => "32 bit Windows report", + "exe64.report" => "64 bit Windows report", + "log" => "task log", + "log.old" => "old logs", ); sub InitMoreInfo($) @@ -298,7 +301,7 @@ sub InitMoreInfo($) my $Value = $self->GetParam("f$Key"); my $TaskDir = $StepTask->GetTaskDir(); - foreach my $Log ("log", "log.old") + foreach my $Log (@MILogFiles) { if (!-f "$TaskDir/$Log" or -z "$TaskDir/$Log") { -- 2.17.1

June 21, 2018

[PATCH 2/3] testbot/WineRunTask: Preserve the old WineTest report in case of timeout.

by Francois Gouget

If the task timed out the new report is likely quite incomplete and thus would be a poor substitute for the existing reference WineTest report. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunTask.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index ed190348b..7f92b8858 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -289,7 +289,7 @@ sub WrapUpAndExit($;$$$) $VM->Save(); } - if ($Status eq 'completed' and $Step->Type eq 'suite') + if ($Step->Type eq 'suite' and $Status eq 'completed' and !$TimedOut) { # Keep the old report if the new one is missing if (-f "$TaskDir/log" and !-z "$TaskDir/log") -- 2.17.1

June 21, 2018

[PATCH 1/3] testbot/WineRunTask: Keep the old reference report if the new one is missing.

by Francois Gouget

The WineTest results usually don't change too much from one run to the next so it's better to have a slightly out of date report than nothing at all. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunTask.pl | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index c4ca578e2..ed190348b 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -291,13 +291,20 @@ sub WrapUpAndExit($;$$$) if ($Status eq 'completed' and $Step->Type eq 'suite') { - # Update the reference VM suite results for WineSendLog.pl - my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", - $Step->FileType eq "exe64" ? "64" : "32"); - unlink("$LatestBaseName.log"); - link("$TaskDir/log", "$LatestBaseName.log") if (-f "$TaskDir/log"); - unlink("$LatestBaseName.err"); - link("$TaskDir/err", "$LatestBaseName.err") if (-f "$TaskDir/err"); + # Keep the old report if the new one is missing + if (-f "$TaskDir/log" and !-z "$TaskDir/log") + { + # Update the reference VM suite results for WineSendLog.pl + my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", + $Step->FileType eq "exe64" ? "64" : "32"); + unlink("$LatestBaseName.log"); + link("$TaskDir/log", "$LatestBaseName.log"); + unlink("$LatestBaseName.err"); + if (-f "$TaskDir/err" and !-z "$TaskDir/err") + { + link("$TaskDir/err", "$LatestBaseName.err"); + } + } } my $Result = $VM->Name .": ". $VM->Status ." Status: $Status Failures: ". (defined $TestFailures ? $TestFailures : "unset"); -- 2.17.1

June 21, 2018

[PATCH] testbot/WineRun*: Don't duplicate errors and factorize the log parsing.

by Francois Gouget

Move the task log parsing functions to LogUtils.pm so the WineRun*.pl scripts can share them. Also stop copying the 'Build: *' result lines from the log to the err file. It's up to the GUI to present all the relevant lines when only showing the log summary (which it does now). Finally, there is nothing for the script to do if the patch fails to apply. So even if we did not get the full log or the task timed out somehow, the status of the task should be 'badpatch' and the other errors don't matter. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunBuild.pl | 49 ++++++++---------- testbot/bin/WineRunReconfig.pl | 49 ++++++------------ testbot/lib/WineTestBot/LogUtils.pm | 77 +++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 64 deletions(-) create mode 100644 testbot/lib/WineTestBot/LogUtils.pm diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index 1bc6cffbc..60af32c73 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -46,6 +46,7 @@ use WineTestBot::Jobs; use WineTestBot::PatchUtils; use WineTestBot::VMs; use WineTestBot::Log; +use WineTestBot::LogUtils; use WineTestBot::Engine::Notify; @@ -391,39 +392,29 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60)) Debug(Elapsed($Start), " Retrieving 'Build.log'\n"); if ($TA->GetFile("Build.log", "$TaskDir/log")) { - if (open(my $LogFile, "<", "$TaskDir/log")) + my $Result = ParseTaskLog("$TaskDir/log", "Build"); + if ($Result eq "ok") { - # Collect and analyze the 'Build:' status line(s) - $ErrMessage ||= ""; - foreach my $Line (<$LogFile>) - { - chomp($Line); - next if ($Line !~ /^Build: (.*)$/); - if ($1 eq "ok") - { - # We must have gotten the full log and the build did succeed. - # So forget any prior error. - $NewStatus = "completed"; - $TAError = $ErrMessage = undef; - } - else - { - $NewStatus = ($1 eq "Patch failed to apply") ? "badpatch" : "badbuild"; - # Collect all the build errors (32 bit, 64 bit, etc) - $ErrMessage .= "$1\n"; - } - } - close($LogFile); - - if (!defined $NewStatus) - { - $NewStatus = "badbuild"; - $ErrMessage = "Missing build status line\n"; - } + # We must have gotten the full log and the build did succeed. + # So forget any prior error. + $NewStatus = "completed"; + $TAError = $ErrMessage = undef; + } + elsif ($Result eq "badpatch") + { + # This too is conclusive enough to ignore other errors. + $NewStatus = "badpatch"; + $TAError = $ErrMessage = undef; + } + elsif ($Result =~ s/^nolog://) + { + FatalError("$Result\n", "retry"); } else { - FatalError("Unable to open the build log for reading: $!\n", "retry"); + # If the result line is missing we probably already have an error message + # that explains why. + $NewStatus = "badbuild"; } } elsif (!defined $TAError) diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index d04b78061..2950799d5 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -45,6 +45,7 @@ use WineTestBot::Config; use WineTestBot::Jobs; use WineTestBot::VMs; use WineTestBot::Log; +use WineTestBot::LogUtils; use WineTestBot::Engine::Notify; @@ -373,45 +374,23 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60)) Debug(Elapsed($Start), " Retrieving 'Reconfig.log'\n"); if ($TA->GetFile("Reconfig.log", "$TaskDir/log")) { - if (open(my $LogFile, "<", "$TaskDir/log")) + my $Result = ParseTaskLog("$TaskDir/log", "Reconfig"); + if ($Result eq "ok") { - # Collect and analyze the 'Reconfig:' status line(s). - my $LogErrors; - foreach my $Line (<$LogFile>) - { - chomp($Line); - next if ($Line !~ /^Reconfig: (.*)$/); - # Add the error message or an empty string for 'ok' - $LogErrors = ($LogErrors || "") . ($1 ne "ok" ? "$1\n" : ""); - } - close($LogFile); - - if (!defined $LogErrors) - { - if (!defined $ErrMessage) - { - $NewStatus = "badbuild"; - $ErrMessage = "Missing reconfig status line\n"; - } - # otherwise $ErrMessage probably already explains why the reconfig - # status line is missing - } - elsif ($LogErrors eq "") - { - # We must have gotten the full log and the build did succeed. - # So forget any prior error. - $NewStatus = "completed"; - $TAError = $ErrMessage = undef; - } - else - { - $NewStatus = "badbuild"; - $ErrMessage = $LogErrors . ($ErrMessage || ""); - } + # We must have gotten the full log and the build did succeed. + # So forget any prior error. + $NewStatus = "completed"; + $TAError = $ErrMessage = undef; + } + elsif ($Result =~ s/^nolog://) + { + FatalError("$Result\n", "retry"); } else { - FatalError("Unable to open the build log for reading: $!\n"); + # We should not have badpatch errors and if the result line is missing we + # probably already have an error message that explains why. + $NewStatus = "badbuild"; } } elsif (!defined $TAError) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm new file mode 100644 index 000000000..3e40fbc35 --- /dev/null +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -0,0 +1,77 @@ +# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*- +# Copyright 2018 Francois Gouget +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + +use strict; + +package WineTestBot::LogUtils; + +=head1 NAME + +WineTestBot::LogUtils - Provides functions to parse task logs + +=cut + + +use Exporter 'import'; +our @EXPORT = qw(ParseTaskLog); + + +# +# Task log parser +# + +=pod +=over 12 + +=item C<ParseTaskLog()> + +Returns ok if the task was successful and an error code otherwise. + +=back +=cut + +sub ParseTaskLog($$) +{ + my ($FileName, $ResultPrefix) = @_; + + if (open(my $LogFile, "<", $FileName)) + { + my $Result; + foreach my $Line (<$LogFile>) + { + chomp $Line; + if ($Line =~ /^$ResultPrefix: ok$/) + { + $Result ||= "ok"; + } + elsif ($Line =~ /^$ResultPrefix: Patch failed to apply$/) + { + $Result = "badpatch"; + last; # Should be the last and most specific message + } + elsif ($Line =~ /^$ResultPrefix: /) + { + $Result = "failed"; + } + } + close($LogFile); + return $Result || "missing"; + } + return "nolog:Unable to open the task log for reading: $!"; +} + +1; -- 2.17.1

June 21, 2018

[PATCH] testbot: VMs::FilterHypervisors() is not used anymore so remove it.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/VMs.pm | 7 ------- 1 file changed, 7 deletions(-) diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm index a29eebbba..2f494cb81 100644 --- a/testbot/lib/WineTestBot/VMs.pm +++ b/testbot/lib/WineTestBot/VMs.pm @@ -730,11 +730,4 @@ sub FilterEnabledStatus($) $self->AddFilter("Status", ["dirty", "reverting", "sleeping", "idle", "running", "off", "offline"]); } -sub FilterHypervisors($$) -{ - my ($self, $Hypervisors) = @_; - - $self->AddFilter("VirtURI", $Hypervisors); -} - 1; -- 2.17.1

June 21, 2018

[PATCH] testbot/web: Detect and highlight some more Git errors.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 8cc354e84..410d18691 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -253,9 +253,12 @@ sub GetHtmlLine($$$) $Line =~ / done $258$/ or $Line =~ /: unhandled exception [0-9a-fA-F]{8} at / or $Line =~ /^Unhandled exception: / or + # Git errors + $Line =~ /^CONFLICT / or + $Line =~ /^error: patch failed:/ or + $Line =~ /^error: corrupt patch / or # Build errors $Line =~ /: error: / or - $Line =~ /^error: patch failed:/ or $Line =~ /^Makefile:[0-9]+: recipe for target .* failed$/ or $Line =~ /^(?:Build|Reconfig|Task): (?!ok)/ or # Typical perl errors -- 2.17.1

June 21, 2018

[PATCH 2/2] testbot/web: Prettify the log / error separator.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 7 ++++++- testbot/web/WineTestBot.css | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 1536e87cd..8cc354e84 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -495,7 +495,12 @@ sub GenerateBody($) chomp $Line; if ($ErrFirst) { - print "<hr>\n" if (!$LogFirst); + if (!$LogFirst) + { + print "<div class='HrTitle'>". + ($ErrName eq "err" ? "Other errors" : "Old errors") . + "<div class='HrLine'></div></div>\n"; + } print "<pre><code>"; $ErrFirst = 0; } diff --git a/testbot/web/WineTestBot.css b/testbot/web/WineTestBot.css index 28ac1b1c2..593c38f44 100644 --- a/testbot/web/WineTestBot.css +++ b/testbot/web/WineTestBot.css @@ -317,6 +317,20 @@ h2 padding-bottom: 0.1em; } +.HrTitle { + margin-top: 0.5em; + display: flex; + align-items: center; + color: #a50d0d; +} + +.HrLine { + border-top: 1px solid grey; + flex-grow: 1; + margin: 0 10px; + border-color: #a50d0d; +} + details { font-style: italic; -- 2.17.1

June 21, 2018

[PATCH 1/2] testbot/web: Better present the logs and task errors.

by Francois Gouget

Add a horizontal ruler so the log / test report is always clearly separated from the task error messages. Avoid duplicating the code that shows the task errors. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 152 +++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 85 deletions(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 3add52b4c..1536e87cd 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -408,127 +408,109 @@ sub GenerateBody($) my $LogName = $MoreInfo->{Full} || $MoreInfo->{Logs}->[0] || "log"; my $ErrName = $LogName eq "log.old" ? "err.old" : "err"; - if (open LOGFILE, "<", "$TaskDir/$LogName") + my ($EmptyDiag, $LogFirst); + if (open(my $LogFile, "<", "$TaskDir/$LogName")) { - my $HasLogEntries = !1; - my $First = 1; + my $HasLogEntries; my $CurrentDll = ""; my $PrintedDll = ""; - my $Line; - while (defined($Line = <LOGFILE>)) + $LogFirst = 1; + foreach my $Line (<$LogFile>) { $HasLogEntries = 1; - chomp($Line); + chomp $Line; if ($Line =~ m/^([^:]+):[^ ]+ start [^ ]+ -\s*$/) { $CurrentDll = $1; } my ($Highlight, $Plain) = $self->GetHtmlLine($MoreInfo->{Full}, $Line); - if ($MoreInfo->{Full} || defined $Highlight) + next if (!$MoreInfo->{Full} and !defined $Highlight); + + if ($PrintedDll ne $CurrentDll && !$MoreInfo->{Full}) + { + print "</code></pre>" if (!$LogFirst); + print "<div class='LogDllName'>$CurrentDll:</div><pre><code>"; + $PrintedDll = $CurrentDll; + $LogFirst = 0; + } + elsif ($LogFirst) + { + print "<pre><code>"; + $LogFirst = 0; + } + if (!$MoreInfo->{Full} && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) + { + my $Unit = $1 ne "" ? "$1: " : ""; + print "${Unit}Timeout\n"; + } + else { - if ($PrintedDll ne $CurrentDll && !$MoreInfo->{Full}) - { - if ($First) - { - $First = !1; - } - else - { - print "</code></pre>"; - } - print "<div class='LogDllName'>$CurrentDll:</div><pre><code>"; - $PrintedDll = $CurrentDll; - } - elsif ($First) - { - print "<pre><code>"; - $First = !1; - } - if (!$MoreInfo->{Full} && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) - { - my $Unit = $1 ne "" ? "$1: " : ""; - print "${Unit}Timeout\n"; - } - else - { - print(($Highlight || $Plain), "\n"); - } + print(($Highlight || $Plain), "\n"); } } - close LOGFILE; + close($LogFile); - if (open ERRFILE, "<", "$TaskDir/$ErrName") + if (!$LogFirst) { - $CurrentDll = "*err*"; - while (defined($Line = <ERRFILE>)) + print "</code></pre>\n"; + } + elsif ($HasLogEntries) + { + # Here we know we did not show the full log since it was not empty, + # and yet we did not show anything to the user. But don't claim there + # is no failure if the error log is not empty. + if (-z "$TaskDir/$ErrName") { - $HasLogEntries = 1; - chomp($Line); - if ($PrintedDll ne $CurrentDll) - { - if ($First) - { - $First = !1; - } - else - { - print "</code></pre>\n"; - } - print "<br><pre><code>"; - $PrintedDll = $CurrentDll; - } - print $self->escapeHTML($Line), "\n"; + print "No ". ($StepTask->Type eq "single" || + $StepTask->Type eq "suite" ? "test" : "build") . + " failures found"; + $LogFirst = 0; } - close ERRFILE; } - - if (! $First) + elsif ($StepTask->Status eq "canceled") { - print "</code></pre>\n"; + $EmptyDiag = "<p>No log, task was canceled</p>\n"; + } + elsif ($StepTask->Status eq "skipped") + { + $EmptyDiag = "<p>No log, task skipped</p>\n"; } else { - print $HasLogEntries ? "No " . - ($StepTask->Type eq "single" || - $StepTask->Type eq "suite" ? "test" : "build") . - " failures found" : "Empty log"; + print "Empty log"; + $LogFirst = 0; } } - elsif (open ERRFILE, "<", "$TaskDir/$ErrName") + else + { + print "No log". ($StepTask->Status =~ /^(?:queued|running)$/ ? " yet" : ""); + $LogFirst = 0; + } + + if (open(my $ErrFile, "<", "$TaskDir/$ErrName")) { - my $HasErrEntries = !1; - my $Line; - while (defined($Line = <ERRFILE>)) + my $ErrFirst = 1; + foreach my $Line (<$ErrFile>) { - chomp($Line); - if (! $HasErrEntries) + chomp $Line; + if ($ErrFirst) { + print "<hr>\n" if (!$LogFirst); print "<pre><code>"; - $HasErrEntries = 1; + $ErrFirst = 0; } print $self->escapeHTML($Line), "\n"; } - if ($HasErrEntries) + close($ErrFile); + + if (!$ErrFirst) { print "</code></pre>\n"; } - else + elsif (defined $EmptyDiag) { - print "Empty log"; + print $EmptyDiag; } - close ERRFILE; - } - elsif ($StepTask->Status eq "canceled") - { - print "<p>No log, task was canceled</p>\n"; - } - elsif ($StepTask->Status eq "skipped") - { - print "<p>No log, task skipped</p>\n"; - } - else - { - print "<p>No log available yet</p>\n"; } } print "</div>\n"; -- 2.17.1

June 21, 2018

Re: [PATCH 2/2] msvcrt/tests: Use the available ARRAY_SIZE() macro

by Piotr Caban

Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>

June 21, 2018

Re: [PATCH 1/2] msvcrt: Rename a macro to ARRAY_SIZE()

by Piotr Caban

Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>

June 21, 2018

Re: [PATCH v8 5/9] ntdll: Return BIOS info from NtQuerySystemInformation on Linux

by Alex Henrie

On Wed, Jun 20, 2018 at 11:59 PM Marvin <testbot(a)winehq.org> wrote: > > Hi, > > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > https://testbot.winehq.org/JobDetails.pl?Key=39269 > > Your paranoid android. > > > === wxppro (32 bit info) === > The task timed out This is an intermittent failure which could not have been caused by this patch's minor changes to the tests: https://testbot.winehq.org/JobDetails.pl?Key=39279 -Alex

June 21, 2018

Re: [PATCH 1/2] iphlpapi: Use a standalone buffer in IcmpSendEcho().

by Huw Davies

On Thu, Jun 21, 2018 at 05:51:13PM +0800, Zhiyi Zhang wrote: > > > On Thu 6 21 17:35, Huw Davies wrote: > > On Fri, Jun 15, 2018 at 12:23:23PM +0800, Zhiyi Zhang wrote: > >> Fix https://bugs.winehq.org/show_bug.cgi?id=43252 > >> > >> The old implementation uses user provided buffer to receive > >> packet data, which is alway not enough, causing data corruptions > >> or incorrectly timeout. > >> > >> Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> > >> --- > >> dlls/iphlpapi/icmp.c | 38 +++++++++++++++++++++++++++------- > >> dlls/iphlpapi/tests/iphlpapi.c | 21 +++++++++++++++---- > >> 2 files changed, 47 insertions(+), 12 deletions(-) > >> > >> diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c > >> index ebc2f2b65c..7c91443598 100644 > >> --- a/dlls/iphlpapi/icmp.c > >> +++ b/dlls/iphlpapi/icmp.c > >> @@ -113,6 +113,9 @@ typedef struct { > >> #define IP_OPTS_DEFAULT 1 > >> #define IP_OPTS_CUSTOM 2 > >> > >> +#define MAXIPLEN 60 > >> +#define MAXICMPLEN 76 > > > > Out of interest, how did you get to 76? > > > > That was copied from https://github.com/iputils/iputils/blob/master/ping.c#L76 I guess it's for errors: 8 for the icmp header, 60 for the original ip header, then 8 for the original icmp header. We probably don't need all of that, but it doesn't hurt. Huw.

June 21, 2018

Re: [PATCH 1/2] iphlpapi: Use a standalone buffer in IcmpSendEcho().

by Zhiyi Zhang

On Thu 6 21 17:35, Huw Davies wrote: > On Fri, Jun 15, 2018 at 12:23:23PM +0800, Zhiyi Zhang wrote: >> Fix https://bugs.winehq.org/show_bug.cgi?id=43252 >> >> The old implementation uses user provided buffer to receive >> packet data, which is alway not enough, causing data corruptions >> or incorrectly timeout. >> >> Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> >> --- >> dlls/iphlpapi/icmp.c | 38 +++++++++++++++++++++++++++------- >> dlls/iphlpapi/tests/iphlpapi.c | 21 +++++++++++++++---- >> 2 files changed, 47 insertions(+), 12 deletions(-) >> >> diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c >> index ebc2f2b65c..7c91443598 100644 >> --- a/dlls/iphlpapi/icmp.c >> +++ b/dlls/iphlpapi/icmp.c >> @@ -113,6 +113,9 @@ typedef struct { >> #define IP_OPTS_DEFAULT 1 >> #define IP_OPTS_CUSTOM 2 >> >> +#define MAXIPLEN 60 >> +#define MAXICMPLEN 76 > > Out of interest, how did you get to 76? > That was copied from https://github.com/iputils/iputils/blob/master/ping.c#L76 >> + >> /* The sequence number is unique process wide, so that all threads >> * have a distinct sequence number. >> */ >> @@ -270,13 +273,14 @@ DWORD WINAPI IcmpSendEcho( >> icmp_t* icp=(icmp_t*)IcmpHandle; >> unsigned char* reqbuf; >> int reqsize; >> + unsigned char* repbuf; >> + int repsize; >> >> struct icmp_echo_reply* ier; >> struct ip* ip_header; >> struct icmp* icmp_header; >> char* endbuf; >> int ip_header_len; >> - int maxlen; >> struct pollfd fdr; >> DWORD send_time,recv_time; >> struct sockaddr_in addr; >> @@ -312,6 +316,16 @@ DWORD WINAPI IcmpSendEcho( >> return 0; >> } >> >> + /* max ip header + max icmp header and error data + reply size(max 65535 on Windows) */ >> + /* FIXME: request size of 65535 is not supported yet because max buffer size of raw socket on linux is 32767 */ >> + repsize=MAXIPLEN+MAXICMPLEN+(ReplySize&0xFFFF); >> + repbuf=HeapAlloc(GetProcessHeap(), 0, repsize); >> + if (reqbuf==NULL) { > > This should be repbuf. > >> + HeapFree(GetProcessHeap(), 0, reqbuf); >> + SetLastError(ERROR_OUTOFMEMORY); >> + return 0; >> + } >> + >> icmp_header=(struct icmp*)reqbuf; >> icmp_header->icmp_type=ICMP_ECHO; >> icmp_header->icmp_code=0; >> @@ -367,9 +381,7 @@ DWORD WINAPI IcmpSendEcho( >> fdr.events = POLLIN; >> addrlen=sizeof(addr); >> ier=ReplyBuffer; >> - ip_header=(struct ip *) ((char *) ReplyBuffer+sizeof(ICMP_ECHO_REPLY)); >> endbuf=(char *) ReplyBuffer+ReplySize; >> - maxlen=ReplySize-sizeof(ICMP_ECHO_REPLY); >> >> /* Send the packet */ >> TRACE("Sending %d bytes (RequestSize=%d) to %s\n", reqsize, RequestSize, inet_ntoa(addr.sin_addr)); >> @@ -407,10 +419,11 @@ DWORD WINAPI IcmpSendEcho( >> } >> >> /* Get the reply */ >> + ip_header=(struct ip*)repbuf; >> ip_header_len=0; /* because gcc was complaining */ >> while (poll(&fdr,1,Timeout)>0) { >> recv_time = GetTickCount(); >> - res=recvfrom(icp->sid, (char*)ip_header, maxlen, 0, (struct sockaddr*)&addr,&addrlen); >> + res=recvfrom(icp->sid, (char*)repbuf, repsize, 0, (struct sockaddr*)&addr, &addrlen); >> TRACE("received %d bytes from %s\n",res, inet_ntoa(addr.sin_addr)); >> ier->Status=IP_REQ_TIMED_OUT; >> >> @@ -508,6 +521,12 @@ DWORD WINAPI IcmpSendEcho( >> else Timeout = 0; >> continue; >> } else { >> + /* Check free space, should be large enough for an ICMP_ECHO_REPLY and remainning icmp data */ >> + if (endbuf-(char *)ier < sizeof(struct icmp_echo_reply)+(res-ip_header_len-ICMP_MINLEN)) { >> + res=ier-(ICMP_ECHO_REPLY *)ReplyBuffer; >> + SetLastError(IP_GENERAL_FAILURE); >> + goto done; >> + } >> /* This is a reply to our packet */ >> memcpy(&ier->Address,&ip_header->ip_src,sizeof(IPAddr)); >> /* Status is already set */ >> @@ -515,7 +534,7 @@ DWORD WINAPI IcmpSendEcho( >> ier->DataSize=res-ip_header_len-ICMP_MINLEN; >> ier->Reserved=0; >> ier->Data=endbuf-ier->DataSize; >> - memmove(ier->Data,((char*)ip_header)+ip_header_len+ICMP_MINLEN,ier->DataSize); >> + memcpy(ier->Data, ((char *)ip_header)+ip_header_len+ICMP_MINLEN, ier->DataSize); >> ier->Options.Ttl=ip_header->ip_ttl; >> ier->Options.Tos=ip_header->ip_tos; >> ier->Options.Flags=ip_header->ip_off >> 13; >> @@ -523,7 +542,7 @@ DWORD WINAPI IcmpSendEcho( >> if (ier->Options.OptionsSize!=0) { >> ier->Options.OptionsData=(unsigned char *) ier->Data-ier->Options.OptionsSize; >> /* FIXME: We are supposed to rearrange the option's 'source route' data */ >> - memmove(ier->Options.OptionsData,((char*)ip_header)+ip_header_len,ier->Options.OptionsSize); >> + memcpy(ier->Options.OptionsData, ((char *)ip_header)+ip_header_len, ier->Options.OptionsSize); >> endbuf=(char*)ier->Options.OptionsData; >> } else { >> ier->Options.OptionsData=NULL; >> @@ -531,9 +550,8 @@ DWORD WINAPI IcmpSendEcho( >> } >> >> /* Prepare for the next packet */ >> + endbuf-=ier->DataSize; >> ier++; >> - ip_header=(struct ip*)(((char*)ip_header)+sizeof(ICMP_ECHO_REPLY)); >> - maxlen=endbuf-(char*)ip_header; >> >> /* Check out whether there is more but don't wait this time */ >> Timeout=0; >> @@ -542,6 +560,10 @@ DWORD WINAPI IcmpSendEcho( >> res=ier-(ICMP_ECHO_REPLY*)ReplyBuffer; >> if (res==0) >> SetLastError(IP_REQ_TIMED_OUT); >> + else >> + SetLastError(NO_ERROR); > > This hunk looks like a separate change so should be a separate patch. > Note, in general setting last error to NO_ERROR looks wrong, though it may > be needed in this case. > > Huw. >

June 21, 2018

[PATCH 2/2] vulkan-1/tests: Add tests.

by Józef Kucia

Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- configure | 1 + configure.ac | 1 + dlls/vulkan-1/Makefile.in | 1 + dlls/vulkan-1/tests/Makefile.in | 5 +++ dlls/vulkan-1/tests/vulkan.c | 92 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+) create mode 100644 dlls/vulkan-1/tests/Makefile.in create mode 100644 dlls/vulkan-1/tests/vulkan.c diff --git a/configure b/configure index bcb660d56982..ddd6db152cc9 100755 --- a/configure +++ b/configure @@ -19222,6 +19222,7 @@ wine_fn_config_makefile dlls/vnetbios.vxd enable_win16 wine_fn_config_makefile dlls/vssapi enable_vssapi wine_fn_config_makefile dlls/vtdapi.vxd enable_win16 wine_fn_config_makefile dlls/vulkan-1 enable_vulkan_1 +wine_fn_config_makefile dlls/vulkan-1/tests enable_tests wine_fn_config_makefile dlls/vwin32.vxd enable_win16 wine_fn_config_makefile dlls/w32skrnl enable_win16 wine_fn_config_makefile dlls/w32sys.dll16 enable_win16 diff --git a/configure.ac b/configure.ac index 469fa8d72977..68f7f146ca1f 100644 --- a/configure.ac +++ b/configure.ac @@ -3721,6 +3721,7 @@ WINE_CONFIG_MAKEFILE(dlls/vnetbios.vxd,enable_win16) WINE_CONFIG_MAKEFILE(dlls/vssapi) WINE_CONFIG_MAKEFILE(dlls/vtdapi.vxd,enable_win16) WINE_CONFIG_MAKEFILE(dlls/vulkan-1) +WINE_CONFIG_MAKEFILE(dlls/vulkan-1/tests) WINE_CONFIG_MAKEFILE(dlls/vwin32.vxd,enable_win16) WINE_CONFIG_MAKEFILE(dlls/w32skrnl,enable_win16) WINE_CONFIG_MAKEFILE(dlls/w32sys.dll16,enable_win16) diff --git a/dlls/vulkan-1/Makefile.in b/dlls/vulkan-1/Makefile.in index 4bb8cec41287..2abc54df0e41 100644 --- a/dlls/vulkan-1/Makefile.in +++ b/dlls/vulkan-1/Makefile.in @@ -1,5 +1,6 @@ MODULE = vulkan-1.dll IMPORTS = winevulkan +IMPORTLIB = vulkan-1 C_SRCS = \ vulkan.c diff --git a/dlls/vulkan-1/tests/Makefile.in b/dlls/vulkan-1/tests/Makefile.in new file mode 100644 index 000000000000..c9f9f0727c80 --- /dev/null +++ b/dlls/vulkan-1/tests/Makefile.in @@ -0,0 +1,5 @@ +TESTDLL = vulkan-1.dll +IMPORTS = vulkan-1 + +C_SRCS = \ + vulkan.c diff --git a/dlls/vulkan-1/tests/vulkan.c b/dlls/vulkan-1/tests/vulkan.c new file mode 100644 index 000000000000..67da880fe470 --- /dev/null +++ b/dlls/vulkan-1/tests/vulkan.c @@ -0,0 +1,92 @@ +/* + * Copyright 2018 Józef Kucia for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "windows.h" +#include "wine/heap.h" +#include "wine/vulkan.h" +#include "wine/test.h" + +static VkResult create_instance(uint32_t extension_count, + const char * const *enabled_extensions, VkInstance *vk_instance) +{ + VkInstanceCreateInfo create_info; + + create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; + create_info.pNext = NULL; + create_info.flags = 0; + create_info.pApplicationInfo = NULL; + create_info.enabledLayerCount = 0; + create_info.ppEnabledLayerNames = NULL; + create_info.enabledExtensionCount = extension_count; + create_info.ppEnabledExtensionNames = enabled_extensions; + + return vkCreateInstance(&create_info, NULL, vk_instance); +} + +static void test_enumerate_physical_devices(void) +{ + VkPhysicalDevice *vk_physical_devices; + VkPhysicalDeviceProperties properties; + VkInstance vk_instance; + unsigned int i; + uint32_t count; + VkResult vr; + + if ((vr = create_instance(0, NULL, &vk_instance)) < 0) + { + skip("Failed to create Vulkan instance, vr %d.\n", vr); + return; + } + ok(vr == VK_SUCCESS, "Got unexpected VkResult %d.\n", vr); + + vr = vkEnumeratePhysicalDevices(vk_instance, &count, NULL); + ok(vr == VK_SUCCESS, "Got unexpected VkResult %d.\n", vr); + if (!count) + { + skip("No physical devices.\n"); + vkDestroyInstance(vk_instance, NULL); + return; + } + + trace("Got %u physical device(s).\n", count); + vk_physical_devices = heap_calloc(count, sizeof(*vk_physical_devices)); + ok(!!vk_physical_devices, "Failed to allocate memory.\n"); + vr = vkEnumeratePhysicalDevices(vk_instance, &count, vk_physical_devices); + ok(vr == VK_SUCCESS, "Got unexpected VkResult %d.\n", vr); + + for (i = 0; i < count; ++i) + { + vkGetPhysicalDeviceProperties(vk_physical_devices[i], &properties); + + trace("Device '%s', %#x:%#x, driver version %u.%u.%u (%#x), api version %u.%u.%u.\n", + properties.deviceName, properties.vendorID, properties.deviceID, + VK_VERSION_MAJOR(properties.driverVersion), VK_VERSION_MINOR(properties.driverVersion), + VK_VERSION_PATCH(properties.driverVersion), properties.driverVersion, + VK_VERSION_MAJOR(properties.apiVersion), VK_VERSION_MINOR(properties.apiVersion), + VK_VERSION_PATCH(properties.apiVersion)); + } + + heap_free(vk_physical_devices); + + vkDestroyInstance(vk_instance, NULL); +} + +START_TEST(vulkan) +{ + test_enumerate_physical_devices(); +} -- 2.16.4

June 21, 2018

[PATCH 1/2] wrc: Accept hyphens in identificators.

by Józef Kucia

It is a bit more permissive than it should be, hyphens should be allowed in resource names, but not in filenames. Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- I need it to add tests for vulkan-1. I would like to add some tests for structure conversion bugs. --- tools/wrc/parser.l | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/wrc/parser.l b/tools/wrc/parser.l index 32a231b5a2ea..0f46670288ce 100644 --- a/tools/wrc/parser.l +++ b/tools/wrc/parser.l @@ -402,7 +402,7 @@ static unsigned long xstrtoul(const char *nptr, char **endptr, int base) 0[oO][0-7]+[lL]? { parser_lval.num = xstrtoul(yytext+2, 0, 8); return (yytext[yyleng-1] == 'L' || yytext[yyleng-1] == 'l') ? tLNUMBER : tNUMBER; } -[A-Za-z_0-9./\\]+ { +[A-Za-z_0-9./\\][A-Za-z_0-9./\\\-]* { struct keyword *tok = iskeyword(yytext); if(tok) -- 2.16.4

June 21, 2018

Re: [PATCH 1/2] iphlpapi: Use a standalone buffer in IcmpSendEcho().

by Huw Davies

On Fri, Jun 15, 2018 at 12:23:23PM +0800, Zhiyi Zhang wrote: > Fix https://bugs.winehq.org/show_bug.cgi?id=43252 > > The old implementation uses user provided buffer to receive > packet data, which is alway not enough, causing data corruptions > or incorrectly timeout. > > Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> > --- > dlls/iphlpapi/icmp.c | 38 +++++++++++++++++++++++++++------- > dlls/iphlpapi/tests/iphlpapi.c | 21 +++++++++++++++---- > 2 files changed, 47 insertions(+), 12 deletions(-) > > diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c > index ebc2f2b65c..7c91443598 100644 > --- a/dlls/iphlpapi/icmp.c > +++ b/dlls/iphlpapi/icmp.c > @@ -113,6 +113,9 @@ typedef struct { > #define IP_OPTS_DEFAULT 1 > #define IP_OPTS_CUSTOM 2 > > +#define MAXIPLEN 60 > +#define MAXICMPLEN 76 Out of interest, how did you get to 76? > + > /* The sequence number is unique process wide, so that all threads > * have a distinct sequence number. > */ > @@ -270,13 +273,14 @@ DWORD WINAPI IcmpSendEcho( > icmp_t* icp=(icmp_t*)IcmpHandle; > unsigned char* reqbuf; > int reqsize; > + unsigned char* repbuf; > + int repsize; > > struct icmp_echo_reply* ier; > struct ip* ip_header; > struct icmp* icmp_header; > char* endbuf; > int ip_header_len; > - int maxlen; > struct pollfd fdr; > DWORD send_time,recv_time; > struct sockaddr_in addr; > @@ -312,6 +316,16 @@ DWORD WINAPI IcmpSendEcho( > return 0; > } > > + /* max ip header + max icmp header and error data + reply size(max 65535 on Windows) */ > + /* FIXME: request size of 65535 is not supported yet because max buffer size of raw socket on linux is 32767 */ > + repsize=MAXIPLEN+MAXICMPLEN+(ReplySize&0xFFFF); > + repbuf=HeapAlloc(GetProcessHeap(), 0, repsize); > + if (reqbuf==NULL) { This should be repbuf. > + HeapFree(GetProcessHeap(), 0, reqbuf); > + SetLastError(ERROR_OUTOFMEMORY); > + return 0; > + } > + > icmp_header=(struct icmp*)reqbuf; > icmp_header->icmp_type=ICMP_ECHO; > icmp_header->icmp_code=0; > @@ -367,9 +381,7 @@ DWORD WINAPI IcmpSendEcho( > fdr.events = POLLIN; > addrlen=sizeof(addr); > ier=ReplyBuffer; > - ip_header=(struct ip *) ((char *) ReplyBuffer+sizeof(ICMP_ECHO_REPLY)); > endbuf=(char *) ReplyBuffer+ReplySize; > - maxlen=ReplySize-sizeof(ICMP_ECHO_REPLY); > > /* Send the packet */ > TRACE("Sending %d bytes (RequestSize=%d) to %s\n", reqsize, RequestSize, inet_ntoa(addr.sin_addr)); > @@ -407,10 +419,11 @@ DWORD WINAPI IcmpSendEcho( > } > > /* Get the reply */ > + ip_header=(struct ip*)repbuf; > ip_header_len=0; /* because gcc was complaining */ > while (poll(&fdr,1,Timeout)>0) { > recv_time = GetTickCount(); > - res=recvfrom(icp->sid, (char*)ip_header, maxlen, 0, (struct sockaddr*)&addr,&addrlen); > + res=recvfrom(icp->sid, (char*)repbuf, repsize, 0, (struct sockaddr*)&addr, &addrlen); > TRACE("received %d bytes from %s\n",res, inet_ntoa(addr.sin_addr)); > ier->Status=IP_REQ_TIMED_OUT; > > @@ -508,6 +521,12 @@ DWORD WINAPI IcmpSendEcho( > else Timeout = 0; > continue; > } else { > + /* Check free space, should be large enough for an ICMP_ECHO_REPLY and remainning icmp data */ > + if (endbuf-(char *)ier < sizeof(struct icmp_echo_reply)+(res-ip_header_len-ICMP_MINLEN)) { > + res=ier-(ICMP_ECHO_REPLY *)ReplyBuffer; > + SetLastError(IP_GENERAL_FAILURE); > + goto done; > + } > /* This is a reply to our packet */ > memcpy(&ier->Address,&ip_header->ip_src,sizeof(IPAddr)); > /* Status is already set */ > @@ -515,7 +534,7 @@ DWORD WINAPI IcmpSendEcho( > ier->DataSize=res-ip_header_len-ICMP_MINLEN; > ier->Reserved=0; > ier->Data=endbuf-ier->DataSize; > - memmove(ier->Data,((char*)ip_header)+ip_header_len+ICMP_MINLEN,ier->DataSize); > + memcpy(ier->Data, ((char *)ip_header)+ip_header_len+ICMP_MINLEN, ier->DataSize); > ier->Options.Ttl=ip_header->ip_ttl; > ier->Options.Tos=ip_header->ip_tos; > ier->Options.Flags=ip_header->ip_off >> 13; > @@ -523,7 +542,7 @@ DWORD WINAPI IcmpSendEcho( > if (ier->Options.OptionsSize!=0) { > ier->Options.OptionsData=(unsigned char *) ier->Data-ier->Options.OptionsSize; > /* FIXME: We are supposed to rearrange the option's 'source route' data */ > - memmove(ier->Options.OptionsData,((char*)ip_header)+ip_header_len,ier->Options.OptionsSize); > + memcpy(ier->Options.OptionsData, ((char *)ip_header)+ip_header_len, ier->Options.OptionsSize); > endbuf=(char*)ier->Options.OptionsData; > } else { > ier->Options.OptionsData=NULL; > @@ -531,9 +550,8 @@ DWORD WINAPI IcmpSendEcho( > } > > /* Prepare for the next packet */ > + endbuf-=ier->DataSize; > ier++; > - ip_header=(struct ip*)(((char*)ip_header)+sizeof(ICMP_ECHO_REPLY)); > - maxlen=endbuf-(char*)ip_header; > > /* Check out whether there is more but don't wait this time */ > Timeout=0; > @@ -542,6 +560,10 @@ DWORD WINAPI IcmpSendEcho( > res=ier-(ICMP_ECHO_REPLY*)ReplyBuffer; > if (res==0) > SetLastError(IP_REQ_TIMED_OUT); > + else > + SetLastError(NO_ERROR); This hunk looks like a separate change so should be a separate patch. Note, in general setting last error to NO_ERROR looks wrong, though it may be needed in this case. Huw.

June 21, 2018

[PATCH] kernel32: Implement K32EnumDeviceDrivers and K32GetDeviceDriverBaseNameA (pt. 2)

by Anthony Lauzon

I noticed my submission yesterday was a little undercooked and since I have spent some more time on it, I thought I'd try to submit a (hopefully) proper patch to the list. This is a rough implementation of K32EnumDeviceDrivers and K32GetDeviceDriverBaseNameA.

June 21, 2018

Re: [PATCH v2] user32: Always release menu item in EnableMenuItem

by Huw Davies

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

June 21, 2018

Re: [PATCH] oleaut32: Correct some comments.

by Huw Davies

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

June 21, 2018

Re: [PATCH v8 5/9] ntdll: Return BIOS info from NtQuerySystemInformation on Linux

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=39269 Your paranoid android. === wxppro (32 bit info) === The task timed out

June 21, 2018

[PATCH] ntdll: Implement LdrRegisterDllNotification and LdrUnregisterDllNotification.

by Alistair Leslie-Hughes

From: Michael Müller <michael(a)fds-team.de> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/ntdll/loader.c | 75 +++++++++++++- dlls/ntdll/tests/rtl.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 332 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 3e00ec6..cae9b25 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -40,6 +40,7 @@ #include "wine/library.h" #include "wine/unicode.h" #include "wine/debug.h" +#include "wine/list.h" #include "wine/server.h" #include "ntdll_misc.h" #include "ddk/wdm.h" @@ -71,6 +72,13 @@ static BOOL imports_fixup_done = FALSE; /* set once the imports have been fixed static BOOL process_detaching = FALSE; /* set on process detach to avoid deadlocks with thread detach */ static int free_lib_count; /* recursion depth of LdrUnloadDll calls */ +struct ldr_notification +{ + struct list entry; + PLDR_DLL_NOTIFICATION_FUNCTION callback; + void *context; +}; + static const char * const reason_names[] = { "PROCESS_DETACH", @@ -342,6 +350,29 @@ static ULONG_PTR allocate_stub( const char *dll, const char *name ) static inline ULONG_PTR allocate_stub( const char *dll, const char *name ) { return 0xdeadbeef; } #endif /* __i386__ */ +/* call ldr notifications */ +static void call_ldr_notifications( ULONG reason, LDR_MODULE *module ) +{ + struct ldr_notification *notify, *notify_next; + LDR_DLL_NOTIFICATION_DATA data; + + data.Loaded.Flags = 0; + data.Loaded.FullDllName = &module->FullDllName; + data.Loaded.BaseDllName = &module->BaseDllName; + data.Loaded.DllBase = module->BaseAddress; + data.Loaded.SizeOfImage = module->SizeOfImage; + + LIST_FOR_EACH_ENTRY_SAFE( notify, notify_next, &ldr_notifications, struct ldr_notification, entry ) + { + TRACE_(relay)("\1Call LDR notification callback (proc=%p,reason=%u,data=%p,context=%p)\n", + notify->callback, reason, &data, notify->context ); + + notify->callback(reason, &data, notify->context); + + TRACE_(relay)("\1Ret LDR notification callback (proc=%p,reason=%u,data=%p,context=%p)\n", + notify->callback, reason, &data, notify->context ); + } +} /************************************************************************* * get_modref @@ -1275,12 +1306,18 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved ) { WINE_MODREF *prev = current_modref; current_modref = wm; + + call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_LOADED, &wm->ldr ); status = MODULE_InitDLL( wm, DLL_PROCESS_ATTACH, lpReserved ); if (status == STATUS_SUCCESS) + { wm->ldr.Flags |= LDR_PROCESS_ATTACHED; + } else { MODULE_InitDLL( wm, DLL_PROCESS_DETACH, lpReserved ); + call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_UNLOADED, &wm->ldr ); + /* point to the name so LdrInitializeThunk can print it */ last_failed_modref = wm; WARN("Initialization of %s failed\n", debugstr_w(wm->ldr.BaseDllName.Buffer)); @@ -1353,6 +1390,7 @@ static void process_detach(void) mod->Flags &= ~LDR_PROCESS_ATTACHED; MODULE_InitDLL( CONTAINING_RECORD(mod, WINE_MODREF, ldr), DLL_PROCESS_DETACH, ULongToPtr(process_detaching) ); + call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_UNLOADED, mod ); /* Restart at head of WINE_MODREF list, as entries might have been added and/or removed while performing the call ... */ @@ -1467,8 +1505,27 @@ NTSTATUS WINAPI LdrEnumerateLoadedModules( void *unknown, LDRENUMPROC callback, NTSTATUS WINAPI LdrRegisterDllNotification(ULONG flags, PLDR_DLL_NOTIFICATION_FUNCTION callback, void *context, void **cookie) { - FIXME( "(%04x, %p, %p, %p) stub\n", flags, callback, context, cookie ); - return STATUS_NOT_IMPLEMENTED; + struct ldr_notification *notify; + + TRACE( "(%x, %p, %p, %p)\n", flags, callback, context, cookie ); + + if (!callback || !cookie) + return STATUS_INVALID_PARAMETER; + + if (flags) + FIXME( "ignoring flags %x\n", flags ); + + notify = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*notify) ); + if (!notify) return STATUS_NO_MEMORY; + notify->callback = callback; + notify->context = context; + + RtlEnterCriticalSection( &loader_section ); + list_add_tail( &ldr_notifications, &notify->entry ); + RtlLeaveCriticalSection( &loader_section ); + + *cookie = notify; + return STATUS_SUCCESS; } /****************************************************************** @@ -1476,8 +1533,18 @@ NTSTATUS WINAPI LdrRegisterDllNotification(ULONG flags, PLDR_DLL_NOTIFICATION_FU */ NTSTATUS WINAPI LdrUnregisterDllNotification( void *cookie ) { - FIXME( "(%p) stub\n", cookie ); - return STATUS_NOT_IMPLEMENTED; + struct ldr_notification *notify = cookie; + + TRACE( "(%p)\n", cookie ); + + if (!notify) return STATUS_INVALID_PARAMETER; + + RtlEnterCriticalSection( &loader_section ); + list_remove( &notify->entry ); + RtlLeaveCriticalSection( &loader_section ); + + RtlFreeHeap( GetProcessHeap(), 0, notify ); + return STATUS_SUCCESS; } /****************************************************************** diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index 2e3a7de..1a9e313 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -101,6 +101,8 @@ static NTSTATUS (WINAPI *pRtlInitializeCriticalSectionEx)(CRITICAL_SECTION *, U static NTSTATUS (WINAPI *pLdrEnumerateLoadedModules)(void *, void *, void *); static NTSTATUS (WINAPI *pRtlMakeSelfRelativeSD)(PSECURITY_DESCRIPTOR,PSECURITY_DESCRIPTOR,LPDWORD); static NTSTATUS (WINAPI *pRtlAbsoluteToSelfRelativeSD)(PSECURITY_DESCRIPTOR,PSECURITY_DESCRIPTOR,PULONG); +static NTSTATUS (WINAPI *pLdrRegisterDllNotification)(ULONG, PLDR_DLL_NOTIFICATION_FUNCTION, void *, void **); +static NTSTATUS (WINAPI *pLdrUnregisterDllNotification)(void *); static HMODULE hkernel32 = 0; static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL); @@ -108,10 +110,15 @@ static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL); #define LEN 16 static const char* src_src = "This is a test!"; /* 16 bytes long, incl NUL */ +static WCHAR ws2_32dllW[] = {'w','s','2','_','3','2','.','d','l','l',0}; +static WCHAR nsidllW[] = {'n','s','i','.','d','l','l',0}; +static WCHAR wintrustdllW[] = {'w','i','n','t','r','u','s','t','.','d','l','l',0}; +static WCHAR crypt32dllW[] = {'c','r','y','p','t','3','2','.','d','l','l',0}; static ULONG src_aligned_block[4]; static ULONG dest_aligned_block[32]; static const char *src = (const char*)src_aligned_block; static char* dest = (char*)dest_aligned_block; +const WCHAR *expected_dll = nsidllW; static void InitFunctionPtrs(void) { @@ -157,6 +164,8 @@ static void InitFunctionPtrs(void) pLdrEnumerateLoadedModules = (void *)GetProcAddress(hntdll, "LdrEnumerateLoadedModules"); pRtlMakeSelfRelativeSD = (void *)GetProcAddress(hntdll, "RtlMakeSelfRelativeSD"); pRtlAbsoluteToSelfRelativeSD = (void *)GetProcAddress(hntdll, "RtlAbsoluteToSelfRelativeSD"); + pLdrRegisterDllNotification = (void *)GetProcAddress(hntdll, "LdrRegisterDllNotification"); + pLdrUnregisterDllNotification = (void *)GetProcAddress(hntdll, "LdrUnregisterDllNotification"); } hkernel32 = LoadLibraryA("kernel32.dll"); ok(hkernel32 != 0, "LoadLibrary failed\n"); @@ -2259,6 +2268,257 @@ static void test_RtlMakeSelfRelativeSD(void) ok( status == STATUS_BAD_DESCRIPTOR_FORMAT, "got %08x\n", status ); } +static DWORD (CALLBACK *orig_entry)(HMODULE,DWORD,LPVOID); +static DWORD *dll_main_data; + +static inline void *get_rva( HMODULE module, DWORD va ) +{ + return (void *)((char *)module + va); +} + +static void CALLBACK ldr_notify_callback1(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + const IMAGE_IMPORT_DESCRIPTOR *imports; + const IMAGE_THUNK_DATA *import_list; + IMAGE_THUNK_DATA *thunk_list; + DWORD *calls = context; + LIST_ENTRY *mark; + LDR_MODULE *mod; + ULONG size; + int i, j; + + *calls <<= 4; + *calls |= reason; + + if (!lstrcmpiW(data->Loaded.BaseDllName->Buffer, expected_dll)) + return; + + ok(data->Loaded.Flags == 0, "Expected flags 0, got %x\n", data->Loaded.Flags); + ok(!lstrcmpiW(data->Loaded.BaseDllName->Buffer, expected_dll), "Expected %s, got %s\n", + wine_dbgstr_w(expected_dll), wine_dbgstr_w(data->Loaded.BaseDllName->Buffer)); + ok(!!data->Loaded.DllBase, "Expected non zero base address\n"); + ok(data->Loaded.SizeOfImage, "Expected non zero image size\n"); + + /* expect module to be last module listed in LdrData load order list */ + mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; + mod = CONTAINING_RECORD(mark->Blink, LDR_MODULE, InMemoryOrderModuleList); + ok(mod->BaseAddress == data->Loaded.DllBase, "Expected base address %p, got %p\n", + data->Loaded.DllBase, mod->BaseAddress); + ok(!lstrcmpiW(mod->BaseDllName.Buffer, expected_dll), "Expected %s, got %s\n", + wine_dbgstr_w(expected_dll), wine_dbgstr_w(mod->BaseDllName.Buffer)); + + /* show that imports have already been resolved */ + imports = RtlImageDirectoryEntryToData(data->Loaded.DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_IMPORT, &size); + ok(!!imports, "Expected dll to have imports\n"); + + for (i = 0; imports[i].Name; i++) + { + thunk_list = get_rva(data->Loaded.DllBase, (DWORD)imports[i].FirstThunk); + if (imports[i].OriginalFirstThunk) + import_list = get_rva(data->Loaded.DllBase, (DWORD)imports[i].OriginalFirstThunk); + else + import_list = thunk_list; + + for (j = 0; import_list[j].u1.Ordinal; j++) + { + ok(thunk_list[j].u1.AddressOfData > data->Loaded.SizeOfImage, + "Import has not been resolved: %p\n", (void*)thunk_list[j].u1.Function); + } + } +} + +static void CALLBACK ldr_notify_callback2(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + DWORD *calls = context; + *calls <<= 4; + *calls |= reason + 2; +} + +static BOOL WINAPI fake_dll_main(HINSTANCE instance, DWORD reason, void* reserved) +{ + if (reason == DLL_PROCESS_ATTACH) + { + *dll_main_data <<= 4; + *dll_main_data |= 3; + } + else if (reason == DLL_PROCESS_DETACH) + { + *dll_main_data <<= 4; + *dll_main_data |= 4; + } + return orig_entry(instance, reason, reserved); +} + +static void CALLBACK ldr_notify_callback_dll_main(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + DWORD *calls = context; + LIST_ENTRY *mark; + LDR_MODULE *mod; + + *calls <<= 4; + *calls |= reason; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; + mod = CONTAINING_RECORD(mark->Blink, LDR_MODULE, InMemoryOrderModuleList); + ok(mod->BaseAddress == data->Loaded.DllBase, "Expected base address %p, got %p\n", + data->Loaded.DllBase, mod->BaseAddress); + if (mod->BaseAddress != data->Loaded.DllBase) + return; + + orig_entry = mod->EntryPoint; + mod->EntryPoint = fake_dll_main; + dll_main_data = calls; +} + +static BOOL WINAPI fake_dll_main_fail(HINSTANCE instance, DWORD reason, void* reserved) +{ + if (reason == DLL_PROCESS_ATTACH) + { + *dll_main_data <<= 4; + *dll_main_data |= 3; + } + else if (reason == DLL_PROCESS_DETACH) + { + *dll_main_data <<= 4; + *dll_main_data |= 4; + } + return FALSE; +} + +static void CALLBACK ldr_notify_callback_fail(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + DWORD *calls = context; + LIST_ENTRY *mark; + LDR_MODULE *mod; + + *calls <<= 4; + *calls |= reason; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; + mod = CONTAINING_RECORD(mark->Blink, LDR_MODULE, InMemoryOrderModuleList); + ok(mod->BaseAddress == data->Loaded.DllBase, "Expected base address %p, got %p\n", + data->Loaded.DllBase, mod->BaseAddress); + if (mod->BaseAddress != data->Loaded.DllBase) + return; + + orig_entry = mod->EntryPoint; + mod->EntryPoint = fake_dll_main_fail; + dll_main_data = calls; +} + +static void CALLBACK ldr_notify_callback_imports(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + DWORD *calls = context; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + if (!lstrcmpiW(data->Loaded.BaseDllName->Buffer, crypt32dllW)) + { + *calls <<= 4; + *calls |= 1; + } + + if (!lstrcmpiW(data->Loaded.BaseDllName->Buffer, wintrustdllW)) + { + *calls <<= 4; + *calls |= 2; + } +} + +static void test_LdrRegisterDllNotification(void) +{ + void *cookie, *cookie2; + NTSTATUS status; + HMODULE mod; + DWORD calls; + + if (!pLdrRegisterDllNotification || !pLdrUnregisterDllNotification) + { + win_skip("Ldr(Un)RegisterDllNotification not available\n"); + return; + } + + mod = LoadLibraryW(expected_dll); + if(mod) + FreeLibrary(mod); + else + expected_dll = ws2_32dllW; /* XP Default */ + + /* generic test */ + status = pLdrRegisterDllNotification(0, ldr_notify_callback1, &calls, &cookie); + ok(!status, "Expected STATUS_SUCCESS, got %08x\n", status); + + calls = 0; + mod = LoadLibraryW(expected_dll); + ok(!!mod, "Failed to load library: %d\n", GetLastError()); + ok(calls == LDR_DLL_NOTIFICATION_REASON_LOADED, "Expected LDR_DLL_NOTIFICATION_REASON_LOADED, got %x\n", calls); + + calls = 0; + FreeLibrary(mod); + ok(calls == LDR_DLL_NOTIFICATION_REASON_UNLOADED, "Expected LDR_DLL_NOTIFICATION_REASON_UNLOADED, got %x\n", calls); + + /* test order of callbacks */ + status = pLdrRegisterDllNotification(0, ldr_notify_callback2, &calls, &cookie2); + ok(!status, "Expected STATUS_SUCCESS, got %08x\n", status); + + calls = 0; + mod = LoadLibraryW(expected_dll); + ok(!!mod, "Failed to load library: %d\n", GetLastError()); + ok(calls == 0x13, "Expected order 0x13, got %x\n", calls); + + calls = 0; + FreeLibrary(mod); + ok(calls == 0x24, "Expected order 0x24, got %x\n", calls); + + pLdrUnregisterDllNotification(cookie2); + pLdrUnregisterDllNotification(cookie); + + /* test dll main order */ + status = pLdrRegisterDllNotification(0, ldr_notify_callback_dll_main, &calls, &cookie); + ok(!status, "Expected STATUS_SUCCESS, got %08x\n", status); + + calls = 0; + mod = LoadLibraryW(expected_dll); + ok(!!mod, "Failed to load library: %d\n", GetLastError()); + ok(calls == 0x13, "Expected order 0x13, got %x\n", calls); + + calls = 0; + FreeLibrary(mod); + ok(calls == 0x42, "Expected order 0x42, got %x\n", calls); + + pLdrUnregisterDllNotification(cookie); + + /* test dll main order */ + status = pLdrRegisterDllNotification(0, ldr_notify_callback_fail, &calls, &cookie); + ok(!status, "Expected STATUS_SUCCESS, got %08x\n", status); + + calls = 0; + mod = LoadLibraryW(expected_dll); + ok(!mod, "Expected library to fail loading\n"); + ok(calls == 0x1342, "Expected order 0x1342, got %x\n", calls); + + pLdrUnregisterDllNotification(cookie); + + /* test dll with dependencies */ + status = pLdrRegisterDllNotification(0, ldr_notify_callback_imports, &calls, &cookie); + ok(!status, "Expected STATUS_SUCCESS, got %08x\n", status); + + calls = 0; + mod = LoadLibraryW(wintrustdllW); + ok(!!mod, "Failed to load library: %d\n", GetLastError()); + ok(calls == 0x12 || calls == 0x21, "got %x\n", calls); + + FreeLibrary(mod); + pLdrUnregisterDllNotification(cookie); +} + START_TEST(rtl) { InitFunctionPtrs(); @@ -2293,4 +2553,5 @@ START_TEST(rtl) test_RtlLeaveCriticalSection(); test_LdrEnumerateLoadedModules(); test_RtlMakeSelfRelativeSD(); + test_LdrRegisterDllNotification(); } -- 1.9.1

June 21, 2018

[PATCH 5/5] ntdll: GetLogicalProcessorInformation report LPT_PC_SMT for SMT cores.

by Roderick Colenbrander

Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index 5712e4cc1b..2ca9cd59b0 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1352,7 +1352,8 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT (*pdata)[i].Relationship = rel; (*pdata)[i].ProcessorMask = mask; - /* TODO: set processor core flags */ + if (rel == RelationProcessorCore) + (*pdata)[i].u.ProcessorCore.Flags = count_bits(mask) > 1 ? LTP_PC_SMT : 0; (*pdata)[i].u.Reserved[0] = 0; (*pdata)[i].u.Reserved[1] = id; *len = i+1; -- 2.14.4

June 21, 2018

[PATCH 4/5] ntdll: GetLogicalProcessorInformationEx report LTP_PC_SMT for SMT cores.

by Roderick Colenbrander

Signed-off-by: Roderick Colenbrander <thunderbird2k(a)gmail.com> --- dlls/ntdll/nt.c | 27 +++++++++++++++------------ include/winnt.h | 2 ++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index b16f320957..5712e4cc1b 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -1309,6 +1309,17 @@ static DWORD log_proc_ex_size_plus(DWORD size) return sizeof(LOGICAL_PROCESSOR_RELATIONSHIP) + sizeof(DWORD) + size; } +static DWORD count_bits(ULONG_PTR mask) +{ + DWORD count = 0; + while (mask > 0) + { + mask >>= 1; + count++; + } + return count; +} + /* Store package and core information for a logical processor. Parsing of processor * data may happen in multiple passes; the 'id' parameter is then used to locate * previously stored data. The type of data stored in 'id' depends on 'rel': @@ -1377,7 +1388,10 @@ static inline BOOL logical_proc_info_add_by_id(SYSTEM_LOGICAL_PROCESSOR_INFORMAT dataex->Relationship = rel; dataex->Size = log_proc_ex_size_plus(sizeof(PROCESSOR_RELATIONSHIP)); - dataex->u.Processor.Flags = 0; /* TODO */ + if (rel == RelationProcessorCore) + dataex->u.Processor.Flags = count_bits(mask) > 1 ? LTP_PC_SMT : 0; + else + dataex->u.Processor.Flags = 0; dataex->u.Processor.EfficiencyClass = 0; dataex->u.Processor.GroupCount = 1; dataex->u.Processor.GroupMask[0].Mask = mask; @@ -1520,17 +1534,6 @@ static inline BOOL logical_proc_info_add_group(SYSTEM_LOGICAL_PROCESSOR_INFORMAT } #ifdef linux -static DWORD count_bits(ULONG_PTR mask) -{ - DWORD count = 0; - while (mask > 0) - { - mask >>= 1; - count++; - } - return count; -} - /* for 'data', max_len is the array count. for 'dataex', max_len is in bytes */ static NTSTATUS create_logical_proc_info(SYSTEM_LOGICAL_PROCESSOR_INFORMATION **data, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX **dataex, DWORD *max_len) diff --git a/include/winnt.h b/include/winnt.h index 54bf11dabd..862cc45291 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -5982,6 +5982,8 @@ typedef enum _LOGICAL_PROCESSOR_RELATIONSHIP RelationAll = 0xffff } LOGICAL_PROCESSOR_RELATIONSHIP; +#define LTP_PC_SMT 0x1 + typedef enum _PROCESSOR_CACHE_TYPE { CacheUnified, -- 2.14.4

June 21, 2018