ampr: assign unique IDs to resolved file paths

src/libs/libAmpr.cpp

@@ -52,6 +52,7 @@ struct ResolvedPathInfo {
 
 static std::mutex                                        g_mutex;
 static uint32_t                                          g_next_submission_id = 1;
+static uint32_t                                          g_next_file_id       = 1;
 static std::unordered_map<uint32_t, bool>                g_submissions;
 static std::condition_variable                           g_submission_cv;
 static std::unordered_map<uint32_t, std::string>         g_files;
@@ -92,15 +93,6 @@ static bool WriteGuest(uint64_t addr, const T& value) {
 	return WriteGuestBytes(addr, &value, sizeof(T));
 }
 
-static uint32_t ComputeFileId(const char* guest_path) {
-	uint32_t hash = 2166136261u;
-	for (auto* p = reinterpret_cast<const uint8_t*>(guest_path); p != nullptr && *p != 0; ++p) {
-		hash ^= *p;
-		hash *= 16777619u;
-	}
-	return hash & static_cast<uint32_t>(std::numeric_limits<int32_t>::max());
-}
-
 static int ReadGuestCString(uint64_t addr, char* out, size_t out_size) {
 	if (addr == 0) {
 		return LibKernel::KERNEL_ERROR_EFAULT;
@@ -120,18 +112,6 @@ static int ReadGuestCString(uint64_t addr, char* out, size_t out_size) {
 	return LibKernel::KERNEL_ERROR_ENAMETOOLONG;
 }
 
-static void RegisterHostPathLocked(uint32_t file_id, const std::string& host_path,
-                                   uint64_t file_size, bool is_dir) {
-	g_files[file_id]      = host_path;
-	g_file_sizes[file_id] = is_dir ? 0 : file_size;
-}
-
-static void RegisterHostPath(uint32_t file_id, const std::string& host_path, uint64_t file_size,
-                             bool is_dir) {
-	std::scoped_lock lock(g_mutex);
-	RegisterHostPathLocked(file_id, host_path, file_size, is_dir);
-}
-
 static bool TryGetHostPath(uint32_t file_id, std::string* out) {
 	std::scoped_lock lock(g_mutex);
 	const auto       it = g_files.find(file_id);
@@ -213,7 +193,6 @@ static int ResolveOnePath(const char* guest_path, uint32_t* id, uint64_t* size)
 
 	if (!found) {
 		const auto real_path = LibKernel::FileSystem::GetRealFilename(path);
-		info.file_id         = AprShared::ComputeFileId(guest_path);
 		info.host_path       = Common::PathToString(real_path);
 
 		if (Common::File::IsDirectoryExisting(real_path)) {
@@ -232,7 +211,14 @@ static int ResolveOnePath(const char* guest_path, uint32_t* id, uint64_t* size)
 			if (!inserted) {
 				info = it->second;
 			} else if (info.result == OK) {
-				RegisterHostPathLocked(info.file_id, info.host_path, info.file_size, info.is_dir);
+				// Allocate once under the cache lock; distinct paths can share a hash.
+				if (g_next_file_id == 0xffffffffu) {
+					info.result = it->second.result = LibKernel::KERNEL_ERROR_ENFILE;
+				} else {
+					info.file_id = it->second.file_id = g_next_file_id++;
+					g_files.emplace(info.file_id, info.host_path);
+					g_file_sizes.emplace(info.file_id, info.is_dir ? 0 : info.file_size);
+				}
 			} else if (info.result == LibKernel::KERNEL_ERROR_ENOENT) {
 				log_missing = true;
 			}
@@ -240,8 +226,6 @@ static int ResolveOnePath(const char* guest_path, uint32_t* id, uint64_t* size)
 		if (log_missing) {
 			LOGF("\tAPR resolve missing path: %s -> %s\n", guest_path, info.host_path.c_str());
 		}
-	} else if (info.result == OK) {
-		AprShared::RegisterHostPath(info.file_id, info.host_path, info.file_size, info.is_dir);
 	}
 
 	if (info.result != OK) {

tests/KernelFileSystemTests.cpp

@@ -857,6 +857,36 @@ void CheckAprPaths(const std::filesystem::path &root) {
             ids[0] == 0xffffffffu && ids[1] == expected_id && sizes[0] == 0 && sizes[1] == 3,
         "APR foreach reports a missing path and continues to the valid file");
 
+  // These paths share the old 31-bit FNV-1a ID (0x122d1544).
+  const char *collision_paths[] = {"perf-audit/test_00015ddf.bin",
+                                   "perf-audit/test_000389b8.bin"};
+  Check(std::filesystem::create_directory(root / "perf-audit"), "create APR collision directory");
+  for (size_t i = 0; i < 2; ++i) {
+    Check(fixture.Create(root / collision_paths[i]), "create APR collision fixture");
+    fixture.Write(i == 0 ? "APR" : "OTHER", i == 0 ? 3 : 5);
+    fixture.Close();
+  }
+  uint32_t collision_ids[2] = {};
+  Check(resolve("/app0/", collision_paths, 2, collision_ids, sizes, &error_index) == OK &&
+            collision_ids[0] != collision_ids[1] && collision_ids[0] != 0xffffffffu &&
+            collision_ids[1] != 0xffffffffu && sizes[0] == 3 && sizes[1] == 5,
+        "APR assigns distinct valid IDs to colliding paths");
+  const auto *stat_symbol = symbols.FindByNid("ApkYaHb8Sek", Loader::SymbolType::Func);
+  const auto *size_symbol = symbols.FindByNid("WvEu7yl3Ivg", Loader::SymbolType::Func);
+  Check(stat_symbol && size_symbol, "APR file metadata exports are registered");
+  using Stat = int (KYTY_SYSV_ABI *)(uint32_t, FileSystem::FileStat *);
+  using Size = int (KYTY_SYSV_ABI *)(uint32_t, uint64_t *);
+  for (size_t i = 0; i < 2; ++i) {
+    FileSystem::FileStat stat {};
+    uint64_t size = 0;
+    Check(resolve("/app0/", &collision_paths[i], 1, ids, nullptr, &error_index) == OK &&
+              ids[0] == collision_ids[i] &&
+              reinterpret_cast<Stat>(stat_symbol->vaddr)(ids[0], &stat) == OK &&
+              reinterpret_cast<Size>(size_symbol->vaddr)(ids[0], &size) == OK &&
+              stat.st_size == static_cast<int64_t>(sizes[i]) && size == sizes[i],
+          "APR re-resolution preserves each file's ID, host path and cached size");
+  }
+
   // PATH_MAX includes NUL; all components remain below NAME_MAX (255).
   std::string longest = "/app0/";
   for (int i = 0; i < 3; ++i) {
@@ -880,7 +910,7 @@ void CheckAprPaths(const std::filesystem::path &root) {
   Check(resolve(unterminated.data(), paths, 1, ids, sizes, &error_index) == -1 &&
             *Libs::Posix::GetErrorAddr() == Libs::Posix::POSIX_ENAMETOOLONG,
         "APR rejects an unterminated prefix");
-  CheckAmprOrdering(symbols, expected_id);
+  CheckAmprOrdering(symbols, collision_ids[0]);
   FileSystem::Umount("/app0");
 }