Skip to content

Commit

Permalink
Fix issue where only part of a process name was being compared
Browse files Browse the repository at this point in the history
The issue was that when a process was identified, it was only being
compared to the first n characters of the -exclude and -process_name
names, where n was the identified process name's length after any
directory and .exe extension had been removed.

e.g., using -exclude code.exe would also cause cod.exe to be excluded
(through, not vice versa).

See issue #172.
  • Loading branch information
JeffersonMontgomery-Intel committed Sep 11, 2023
1 parent 3cc6971 commit 017d222
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 38 deletions.
24 changes: 12 additions & 12 deletions PresentMon/CommandLine.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2017,2019-2022 Intel Corporation
// Copyright (C) 2017,2019-2023 Intel Corporation
// SPDX-License-Identifier: MIT

#include <generated/version.h>
Expand Down Expand Up @@ -244,7 +244,7 @@ bool ParseValue(char** argv, int argc, int* i, char const** value)
return true;
}

bool ParseValue(char** argv, int argc, int* i, std::vector<char const*>* value)
bool ParseValue(char** argv, int argc, int* i, std::vector<std::string>* value)
{
char const* v = nullptr;
if (!ParseValue(argv, argc, i, &v)) return false;
Expand Down Expand Up @@ -554,19 +554,19 @@ bool ParseCommandLine(int argc, char** argv)
" output, and recording arguments.\n");
}

// Prune any directory and ".exe" off of the provided process names. This
// is primarily because the ProcessStart event typically has a full path
// including "\\Device\\..." and ProcessStop event sometimes is missing
// part of the extension.
// Convert the provided process names into a canonical form used for comparison.
// The comparison is not case-sensitive, and does not include any directory nor
// extension.
//
// This is because the different paths for obtaining process information return
// different image name strings. e.g., the ProcessStart event typically has a
// full path including "\\Device\\..." and ProcessStop event sometimes is
// missing part of the extension.
for (auto& name : args->mTargetProcessNames) {
auto pr = GetProcessNameComparisonRange(name, strlen(name));
name += pr.first;
((char*) name)[pr.second] = '\0';
CanonicalizeProcessName(&name);
}
for (auto& name : args->mExcludeProcessNames) {
auto pr = GetProcessNameComparisonRange(name, strlen(name));
name += pr.first;
((char*) name)[pr.second] = '\0';
CanonicalizeProcessName(&name);
}

return true;
Expand Down
39 changes: 17 additions & 22 deletions PresentMon/OutputThread.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2019-2022 Intel Corporation
// Copyright (C) 2019-2023 Intel Corporation
// SPDX-License-Identifier: MIT

#include "PresentMon.hpp"
Expand Down Expand Up @@ -91,40 +91,35 @@ static void UpdateRecordingToggles(size_t nextIndex)
static std::unordered_map<uint32_t, ProcessInfo> gProcesses;
static uint32_t gTargetProcessCount = 0;

std::pair<size_t, size_t> GetProcessNameComparisonRange(char const* name, size_t length)
// Removes any directory and extension, and converts the remaining name to
// lower case.
void CanonicalizeProcessName(std::string* name)
{
std::pair<size_t, size_t> pr(0, length);

if (pr.second >= 4 && _stricmp(name + pr.second - 4, ".exe") == 0) {
pr.second -= 4;
}
for (size_t i = pr.second; i--; ) {
if (name[i] == '/' || name[i] == '\\') {
pr.first = i + 1;
break;
}
size_t i = name->find_last_of("./\\");
if (i != std::string::npos && (*name)[i] == '.') {
name->resize(i);
i = name->find_last_of("/\\");
}

pr.second = pr.second - pr.first;
return pr;
*name = name->substr(i + 1);

std::transform(name->begin(), name->end(), name->begin(),
[](unsigned char c) { return (unsigned char) ::tolower((int) c); });
}

static bool IsTargetProcess(uint32_t processId, std::string const& processName)
{
auto const& args = GetCommandLineArgs();

char const* compareName = nullptr;
size_t compareLength = 0;
std::string compareName;
if (args.mExcludeProcessNames.size() + args.mTargetProcessNames.size() > 0) {
compareName = processName.c_str();
auto pr = GetProcessNameComparisonRange(compareName, processName.size());
compareName += pr.first;
compareLength = pr.second;
compareName = processName;
CanonicalizeProcessName(&compareName);
}

// -exclude
for (auto excludeProcessName : args.mExcludeProcessNames) {
if (_strnicmp(excludeProcessName, compareName, compareLength) == 0) {
if (excludeProcessName == compareName) {
return false;
}
}
Expand All @@ -141,7 +136,7 @@ static bool IsTargetProcess(uint32_t processId, std::string const& processName)

// -process_name
for (auto targetProcessName : args.mTargetProcessNames) {
if (_strnicmp(targetProcessName, compareName, compareLength) == 0) {
if (targetProcessName == compareName) {
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions PresentMon/PresentMon.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2017,2019-2022 Intel Corporation
// Copyright (C) 2017,2019-2023 Intel Corporation
// SPDX-License-Identifier: MIT

#pragma once
Expand Down Expand Up @@ -42,8 +42,8 @@ enum class ConsoleOutput {
};

struct CommandLineArgs {
std::vector<const char*> mTargetProcessNames;
std::vector<const char*> mExcludeProcessNames;
std::vector<std::string> mTargetProcessNames;
std::vector<std::string> mExcludeProcessNames;
const char *mOutputCsvFileName;
const char *mEtlFileName;
const char *mSessionName;
Expand Down Expand Up @@ -137,7 +137,7 @@ void ExitMainThread();
void StartOutputThread();
void StopOutputThread();
void SetOutputRecordingState(bool record);
std::pair<size_t, size_t> GetProcessNameComparisonRange(char const* name, size_t length);
void CanonicalizeProcessName(std::string* path);

// Privilege.cpp:
bool InPerfLogUsersGroup();
Expand Down
41 changes: 41 additions & 0 deletions Tools/run_tests.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ if %do_realtime_tests% EQU 1 (

call :realtime_multicsv_test

call :realtime_exclude_test

echo.
)

Expand Down Expand Up @@ -380,3 +382,42 @@ exit /b 0

exit /b 0

:: -----------------------------------------------------------------------------
:realtime_exclude_test
call :start_target_app /width=320 /height=240
if %errorlevel% NEQ 0 (
echo error: realtime PresentBench tests cannot run with a process named PresentBench.exe already running
set /a errorcount=%errorcount%+1
exit /b 0
)

set saw_excluded=0
set saw_nonexcluded=0
for /f "tokens=1 delims=," %%a in ('"%pmdir%\build\%test_config%\PresentMon-%version%-x64.exe" -exclude presentbench -output_stdout -timed 2 -terminate_after_timed 2^>NUL') do (
if "%%a" EQU "PresentBench.exe" (
set saw_excluded=1
)
)
for /f "tokens=1 delims=," %%a in ('"%pmdir%\build\%test_config%\PresentMon-%version%-x64.exe" -exclude presentbench2 -output_stdout -timed 2 -terminate_after_timed 2^>NUL') do (
if "%%a" EQU "PresentBench.exe" (
set saw_nonexcluded=1
)
)

call :stop_target_app

if %saw_nonexcluded% EQU 0 (
echo error: -exclude PresentBench2 did not record any presents
set /a errorcount=%errorcount%+1
exit /b 0
)
if %saw_excluded% EQU 1 (
echo error: -exclude PresentBench recorded presents
set /a errorcount=%errorcount%+1
exit /b 0
)

echo. -exclude presentbench
echo. -exclude presentbench2
exit /b 0

0 comments on commit 017d222

Please sign in to comment.