microsoft
/
BuildXL
зеркало из https://github.com/microsoft/BuildXL.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Merged PR 798865: Remove UnixAbsentProbe operation 

Use regular probes instead

Related work items: #2182231
This commit is contained in:
Serge Mera 2024-08-06 20:18:41 +00:00
Родитель 47dd1bc10a
Коммит 493d2a932b
12 изменённых файлов: 33 добавлений и 94 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

13
Public/Src/Engine/ProcessPipExecutor/SandboxedProcessPipExecutor.cs
Просмотреть файл

@ -4107,17 +4107,6 @@ namespace BuildXL.ProcessPipExecutor
// the same for all of them. We only need to query one of them.
ReportedFileAccess firstAccess = entry.Value.First();
// Discard entries that have a single UnixAbsentProbe report on a path that contains an intermediate directory symlink.
// Reason: observed accesses computed here should only contain fully expanded paths to avoid ambiguity;
// on Mac, all access reports except for UnixAbsentProbe report fully expanded paths, so only UnixAbsentProbe paths need to be curated.
if (entry.Value.Count == 1
&& firstAccess.Operation == ReportedFileOperation.UnixAbsentProbe
&& firstAccess.ManifestPath.IsValid
&& CheckIfPathContainsSymlinks(firstAccess.ManifestPath.GetParent(m_context.PathTable)))
{
Counters.IncrementCounter(SandboxedProcessCounters.DirectorySymlinkPathsDiscardedCount);
continue;
}
bool isPathCandidateToBeOwnedByASharedOpaque = false;
@ -4207,7 +4196,7 @@ namespace BuildXL.ProcessPipExecutor
// Absent accesses may still contain reparse points. If we are fully resolving them, keep track of them for further processing
if (!hasEnumeration
&& EnableFullReparsePointResolving(m_configuration, m_pip)
&& entry.Value.All(fa => fa.Error == NativeIOConstants.ErrorPathNotFound))
&& entry.Value.All(fa => (fa.Error == NativeIOConstants.ErrorPathNotFound || fa.Error == NativeIOConstants.ErrorFileNotFound)))
{
maybeUnresolvedAbsentAccesses.Add(entry.Key);
}

3
Public/Src/Engine/Processes/FileAccessReportLine.cs
Просмотреть файл

@ -102,8 +102,7 @@ namespace BuildXL.Processes
{ FileOperationLinux.Operations.CreateHardlinkDest.ToString(), ReportedFileOperation.CreateHardlinkDest },
{ FileOperationLinux.Operations.OpenDirectory.ToString(), ReportedFileOperation.OpenDirectory },
{ FileOperationLinux.Operations.Close.ToString(), ReportedFileOperation.Close },
{ FileOperationLinux.Operations.Probe.ToString(), ReportedFileOperation.Probe },
{ FileOperationLinux.Operations.UnixAbsentProbe.ToString(), ReportedFileOperation.UnixAbsentProbe }
{ FileOperationLinux.Operations.Probe.ToString(), ReportedFileOperation.Probe }
};
private static readonly Dictionary<MemoryString, ReportedFileOperation> s_memoryStringBasedOperations =

8
Public/Src/Engine/Processes/FileOperation.Linux.cs
Просмотреть файл

@ -86,13 +86,6 @@ namespace BuildXL.Processes
/// </summary>
Probe,
/// <summary>
/// Represents a read operation for a file that does not exist.
/// </summary>
/// <remarks>
/// This is a special case that was leftover from the macOS sandbox implementation.
/// </remarks>
UnixAbsentProbe,
/// <summary>
/// Maximum value of the enum.
/// </summary>
Max,
@ -122,7 +115,6 @@ namespace BuildXL.Processes
Operations.RemoveDirectory => ReportedFileOperation.RemoveDirectory,
Operations.Close => ReportedFileOperation.Close,
Operations.Probe => ReportedFileOperation.Probe,
Operations.UnixAbsentProbe => ReportedFileOperation.UnixAbsentProbe,
_ => throw new Exception($"Unsupported operation {op}"),
};
}

8
Public/Src/Engine/Processes/ReportedFileOperation.cs
Просмотреть файл

@ -283,13 +283,5 @@ namespace BuildXL.Processes
/// Generic probe file system call.
/// </summary>
Probe,
/// <summary>
/// Represents a read operation for a file that does not exist.
/// </summary>
/// <remarks>
/// Previously named MacLookup.
/// </remarks>
UnixAbsentProbe,
}
}

6
Public/Src/Engine/Processes/SandboxedProcessFactory.cs
Просмотреть файл

@ -67,12 +67,6 @@ namespace BuildXL.Processes
[CounterType(CounterType.Numeric)]
DirectorySymlinkPathsCheckedCount,
/// <summary>
/// Number of paths with directory symlinks that were discarded
/// </summary>
[CounterType(CounterType.Numeric)]
DirectorySymlinkPathsDiscardedCount,
/// <summary>
/// Duration of lazily deleting shared opaque outputs (when enabled)
/// </summary>

15
Public/Src/Engine/Processes/SandboxedProcessUnix.cs
Просмотреть файл

@ -801,21 +801,6 @@ namespace BuildXL.Processes
m_processExitReceived = true;
}
// special handling for UnixAbsentProbe:
// - don't report for existent paths (because for those paths other reports will follow)
// - otherwise, set report.RequestAccess to Probe (because the Sandbox reports 'AbsentProbe', but BXL expects 'Probe'),
if (report.FileOperation == ReportedFileOperation.UnixAbsentProbe)
{
if (FileUtilities.Exists(reportPath))
{
return;
}
else
{
report.RequestedAccess = RequestedAccess.Probe;
}
}
if (report.FileOperation == ReportedFileOperation.ProcessTreeCompletedAck)
{
m_pendingReports.Complete();

2
Public/Src/Engine/UnitTests/Processes/FileAccessExplicitReportingTest.cs
Просмотреть файл

@ -291,7 +291,7 @@ namespace Test.BuildXL.Processes
? new[] { ReportedFileOperation.CreateFile }
: expected.Exists
? new[] { ReportedFileOperation.ReadFile }
: new[] { ReportedFileOperation.UnixAbsentProbe };
: new[] { ReportedFileOperation.Probe };
XAssert.Contains(allowedOperations, actualReport.Operation);
}

4
Public/Src/Engine/UnitTests/Processes/LinuxSandboxProcessBasicTests.cs
Просмотреть файл

@ -893,7 +893,7 @@ namespace Test.BuildXL.Processes
// Retrieve the absent probe that happened under clone3
var probe = result.result.FileAccesses.Single(
access => access.Operation == ReportedFileOperation.UnixAbsentProbe && access.GetPath(Context.PathTable).EndsWith("absentFile"));
access => access.Operation == ReportedFileOperation.Probe && access.GetPath(Context.PathTable).EndsWith("absentFile"));
// The probe should get assigned the parent process path
Assert.Equal(TestProcessExe, probe.Process.Path);
@ -918,7 +918,7 @@ namespace Test.BuildXL.Processes
// Retrieve the directory probe that happened under the (nested) clone3
var probe = result.result.FileAccesses.Single(
access => access.Operation == ReportedFileOperation.UnixAbsentProbe && access.GetPath(Context.PathTable).EndsWith("absentFile"));
access => access.Operation == ReportedFileOperation.Probe && access.GetPath(Context.PathTable).EndsWith("absentFile"));
// The probe should get assigned the ancestor process path
Assert.Equal(TestProcessExe, probe.Process.Path);

54
Public/Src/Sandbox/Linux/AccessChecker.cpp
Просмотреть файл

@ -31,17 +31,17 @@ PolicySearchCursor AccessChecker::FindManifestRecord(const buildxl::common::File
return FindFileAccessPolicyInTreeEx(fam->GetUnixManifestTreeRoot(), path_without_root_sentinel, len);
}
std::tuple<AccessCheckResult, PolicyResult> AccessChecker::GetResult(const buildxl::common::FileAccessManifest *fam, CheckerType checker, const char* path, bool is_directory) {
std::tuple<AccessCheckResult, PolicyResult> AccessChecker::GetResult(const buildxl::common::FileAccessManifest *fam, CheckerType checker, const char* path, bool is_directory, bool exists) {
auto policy = PolicyForPath(fam, path);
AccessCheckResult result = AccessCheckResult::Invalid();
PerformAccessCheck(checker, policy, is_directory, &result);
PerformAccessCheck(checker, policy, is_directory, exists, &result);
return { result, policy };
}
AccessCheckResult AccessChecker::GetAccessCheckAndSetProperties(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event, CheckerType checker) {
auto [result, policy] = GetResult(fam, checker, event.GetSrcPath().c_str(), event.IsDirectory());
auto [result, policy] = GetResult(fam, checker, event.GetSrcPath().c_str(), event.IsDirectory(), event.PathExists());
event.SetSourceAccessCheck(result);
return result;
}
@ -104,14 +104,14 @@ AccessCheckResult AccessChecker::HandleProcessExit(const buildxl::common::FileAc
AccessCheckResult AccessChecker::HandleOpen(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event) {
CheckerType checker = event.PathExists()
? event.IsDirectory() ? CheckerType::kEnumerateDir : CheckerType::kRead
: CheckerType::kUnixAbsentProbe;
: CheckerType::kProbe;
event.SetSourceFileOperation(
event.PathExists()
? event.IsDirectory()
? buildxl::linux::FileOperation::kOpenDirectory
: buildxl::linux::FileOperation::kReadFile
: buildxl::linux::FileOperation::kUnixAbsentProbe
: buildxl::linux::FileOperation::kProbe
);
return GetAccessCheckAndSetProperties(fam, event, checker);
@ -147,8 +147,8 @@ AccessCheckResult AccessChecker::HandleLink(const buildxl::common::FileAccessMan
event.SetSourceFileOperation(buildxl::linux::FileOperation::kCreateHardlinkSource);
event.SetDestinationFileOperation(buildxl::linux::FileOperation::kCreateHardlinkDest);
auto source = GetResult(fam, CheckerType::kRead, event.GetSrcPath().c_str(), event.IsDirectory());
auto destination = GetResult(fam, CheckerType::kWrite, event.GetDstPath().c_str(), event.IsDirectory());
auto source = GetResult(fam, CheckerType::kRead, event.GetSrcPath().c_str(), event.IsDirectory(), event.PathExists());
auto destination = GetResult(fam, CheckerType::kWrite, event.GetDstPath().c_str(), event.IsDirectory(), event.PathExists());
auto combined_access_check = AccessCheckResult::Combine(std::get<0>(source), std::get<0>(destination));
event.SetSourceAccessCheck(std::get<0>(source));
@ -165,8 +165,8 @@ AccessCheckResult AccessChecker::HandleUnlink(const buildxl::common::FileAccessM
}
AccessCheckResult AccessChecker::HandleReadlink(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event) {
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kReadlink : buildxl::linux::FileOperation::kUnixAbsentProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kRead : CheckerType::kUnixAbsentProbe);
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kReadlink : buildxl::linux::FileOperation::kProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kRead : CheckerType::kProbe);
}
AccessCheckResult AccessChecker::HandleRename(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event) {
@ -179,8 +179,8 @@ AccessCheckResult AccessChecker::HandleRename(const buildxl::common::FileAccessM
event.SetDestinationFileOperation(buildxl::linux::FileOperation::kCreateFile);
}
auto source = GetResult(fam, CheckerType::kWrite, event.GetSrcPath().c_str(), event.IsDirectory());
auto destination = GetResult(fam, CheckerType::kWrite, event.GetDstPath().c_str(), event.IsDirectory());
auto source = GetResult(fam, CheckerType::kWrite, event.GetSrcPath().c_str(), event.IsDirectory(), event.PathExists());
auto destination = GetResult(fam, CheckerType::kWrite, event.GetDstPath().c_str(), event.IsDirectory(), event.PathExists());
auto combined_access_check = AccessCheckResult::Combine(std::get<0>(source), std::get<0>(destination));
event.SetSourceAccessCheck(std::get<0>(source));
@ -195,19 +195,19 @@ AccessCheckResult AccessChecker::HandleGenericWrite(const buildxl::common::FileA
}
AccessCheckResult AccessChecker::HandleGenericRead(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event) {
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kReadFile : buildxl::linux::FileOperation::kUnixAbsentProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kRead : CheckerType::kUnixAbsentProbe);
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kReadFile : buildxl::linux::FileOperation::kProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kRead : CheckerType::kProbe);
}
AccessCheckResult AccessChecker::HandleGenericProbe(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event) {
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kProbe : buildxl::linux::FileOperation::kUnixAbsentProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kProbe : CheckerType::kUnixAbsentProbe);
event.SetSourceFileOperation(event.PathExists() ? buildxl::linux::FileOperation::kProbe : buildxl::linux::FileOperation::kProbe);
return GetAccessCheckAndSetProperties(fam, event, event.PathExists() ? CheckerType::kProbe : CheckerType::kProbe);
}
/**
* Checker Functions
*/
void AccessChecker::PerformAccessCheck(CheckerType type, PolicyResult policy, bool is_dir, AccessCheckResult *check_result) {
void AccessChecker::PerformAccessCheck(CheckerType type, PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result) {
switch(type) {
case CheckerType::kExecute:
CheckExecute(policy, is_dir, check_result);
@ -219,10 +219,7 @@ void AccessChecker::PerformAccessCheck(CheckerType type, PolicyResult policy, bo
CheckWrite(policy, is_dir, check_result);
break;
case CheckerType::kProbe:
CheckProbe(policy, is_dir, check_result);
break;
case CheckerType::kUnixAbsentProbe:
CheckUnixAbsentProbe(policy, is_dir, check_result);
CheckProbe(policy, is_dir, exists, check_result);
break;
case CheckerType::kEnumerateDir:
CheckEnumerateDir(policy, is_dir, check_result);
@ -234,7 +231,7 @@ void AccessChecker::PerformAccessCheck(CheckerType type, PolicyResult policy, bo
CheckCreateDirectory(policy, is_dir, check_result);
break;
case CheckerType::kCreateDirectoryNoEnforcement:
CheckCreateDirectoryNoEnforcement(policy, is_dir, check_result);
CheckCreateDirectoryNoEnforcement(policy, is_dir, exists, check_result);
break;
default:
assert(false && "Invalid CheckerType");
@ -250,13 +247,10 @@ void AccessChecker::CheckExecute(PolicyResult policy, bool is_dir, AccessCheckRe
*check_result = policy.CheckReadAccess(requestedAccess, FileReadContext(FileExistence::Existent, is_dir));
}
void AccessChecker::CheckProbe(PolicyResult policy, bool is_dir, AccessCheckResult *check_result) {
*check_result = policy.CheckReadAccess(RequestedReadAccess::Probe, FileReadContext(FileExistence::Existent, is_dir));
}
void AccessChecker::CheckUnixAbsentProbe(PolicyResult policy, bool is_dir, AccessCheckResult *check_result) {
*check_result = policy.CheckReadAccess(RequestedReadAccess::Probe, FileReadContext(FileExistence::Nonexistent));
check_result->Access = RequestedAccess::Lookup;
void AccessChecker::CheckProbe(PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result) {
*check_result = policy.CheckReadAccess(
RequestedReadAccess::Probe,
exists ? FileReadContext(FileExistence::Existent, is_dir) : FileReadContext(FileExistence::Nonexistent));
}
void AccessChecker::CheckRead(PolicyResult policy, bool is_dir, AccessCheckResult *check_result) {
@ -289,11 +283,11 @@ void AccessChecker::CheckCreateDirectory(PolicyResult policy, bool is_dir, Acces
*check_result = policy.CheckCreateDirectoryAccess();
}
void AccessChecker::CheckCreateDirectoryNoEnforcement(PolicyResult policy, bool is_dir, AccessCheckResult *check_result) {
void AccessChecker::CheckCreateDirectoryNoEnforcement(PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result) {
// CODESYNC: CreateDirectoryW in DetouredFunctions.cpp
*check_result = policy.CheckCreateDirectoryAccess();
if (check_result->ShouldDenyAccess()) {
CheckProbe(policy, is_dir, check_result);
CheckProbe(policy, is_dir, exists, check_result);
}
}

10
Public/Src/Sandbox/Linux/AccessChecker.h
Просмотреть файл

@ -21,7 +21,6 @@ enum class CheckerType {
kRead,
kWrite,
kProbe,
kUnixAbsentProbe,
kEnumerateDir,
kCreateSymlink,
kCreateDirectory,
@ -43,7 +42,7 @@ private:
static PolicyResult PolicyForPath(const buildxl::common::FileAccessManifest *fam, const char* absolute_path);
static PolicySearchCursor FindManifestRecord(const buildxl::common::FileAccessManifest *fam, const char* absolute_path, size_t path_length);
static std::tuple<AccessCheckResult, PolicyResult> GetResult(const buildxl::common::FileAccessManifest *fam, CheckerType checker, const char* path, bool is_directory);
static std::tuple<AccessCheckResult, PolicyResult> GetResult(const buildxl::common::FileAccessManifest *fam, CheckerType checker, const char* path, bool is_directory, bool exists);
static AccessCheckResult GetAccessCheckAndSetProperties(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event, CheckerType checker);
/** Handler Functions */
@ -63,16 +62,15 @@ private:
static AccessCheckResult HandleGenericProbe(const buildxl::common::FileAccessManifest *fam, buildxl::linux::SandboxEvent &event);
/** Checker Functions */
static void PerformAccessCheck(CheckerType type, PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void PerformAccessCheck(CheckerType type, PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result);
static void CheckExecute(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckProbe(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckUnixAbsentProbe(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckProbe(PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result);
static void CheckRead(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckEnumerateDir(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckWrite(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckCreateSymlink(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckCreateDirectory(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckCreateDirectoryNoEnforcement(PolicyResult policy, bool is_dir, AccessCheckResult *check_result);
static void CheckCreateDirectoryNoEnforcement(PolicyResult policy, bool is_dir, bool exists, AccessCheckResult *check_result);
};
} // namespace linux

1
Public/Src/Sandbox/Linux/Operations.h
Просмотреть файл

@ -54,7 +54,6 @@ enum class FileOperation {
kRemoveDirectory,
kClose,
kProbe,
kUnixAbsentProbe, // This operation is something leftover from macOS previously named MacLookup that needs to be cleaned up in the future, but is a bit complicated
kMax,
};

3
Public/Src/Utilities/Configuration/UnsafeSandboxConfigurationDefaultExtensions.cs
Просмотреть файл

@ -38,9 +38,6 @@ namespace BuildXL.Utilities.Configuration
/// <summary>
/// Whether <see cref="IUnsafeSandboxConfiguration.EnableFullReparsePointResolving"/> is enabled and we are in a Windows-based OS
/// </summary>
/// <remarks>
/// Mac already resolves all reparse point in its sandbox, and doesn't need post-processing since MacLookup operations are just ignored.
/// </remarks>
public static bool EnableFullReparsePointResolving(this IUnsafeSandboxConfiguration configuration) =>
((configuration.EnableFullReparsePointResolving ?? DefaultEnableFullReparsePointResolving) || !configuration.IgnoreFullReparsePointResolving);