[clang][OpenMP] Fix bug #62099 - use hash value when inode ID cannot be determined

[mjklemm]

When creating the name of an outlined region, Clang uses the file's inode ID to generate a unique name. When the file does not exist, this causes a fatal abort of the compiler. This PR switches to a has value that is used instead.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

Author: Michael Klemm (mjklemm)

Changes

When creating the name of an outlined region, Clang uses the file's inode ID to generate a unique name. When the file does not exist, this causes a fatal abort of the compiler. This PR switches to a has value that is used instead.

---

Full diff: https://github.com/llvm/llvm-project/pull/131646.diff

1 Files Affected:

• (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+10-6)

diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 8dcebcdb8791d..dacbc00ec7819 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -9577,14 +9577,18 @@ OpenMPIRBuilder::getTargetEntryUniqueInfo(FileIdentifierInfoCallbackTy CallBack,
                                           StringRef ParentName) {
   sys::fs::UniqueID ID;
   auto FileIDInfo = CallBack();
-  if (auto EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID)) {
-    report_fatal_error(("Unable to get unique ID for file, during "
-                        "getTargetEntryUniqueInfo, error message: " +
-                        EC.message())
-                           .c_str());
+  uint64_t FileID{0};
+  auto EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID);
+  if (EC) {
+    // The inode ID could not be determined, so create a hash value
+    // the current file name and use that as an ID.
+    FileID = hash_value(FileIDInfo);
+  }
+  else {
+    FileID = ID.getFile();
   }
 
-  return TargetRegionEntryInfo(ParentName, ID.getDevice(), ID.getFile(),
+  return TargetRegionEntryInfo(ParentName, ID.getDevice(), FileID,
                                std::get<1>(FileIDInfo));
 }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Meinersbur]

LLVM coding style applies here:

Suggested change

	uint64_t FileID{0};
	auto EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID);
	if (EC) {
	// The inode ID could not be determined, so create a hash value
	// the current file name and use that as an ID.
	FileID = hash_value(FileIDInfo);
	}
	else {
	FileID = ID.getFile();
	}
	uint64_t FileID = 0;
	// If the inode ID could not be determined, create a hash value
	// the current file name and use that as an ID.
	if (std::error_code EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID) {
	FileID = hash_value(FileIDInfo);
	else
	FileID = ID.getFile();

Or as a lambda:

Suggested change

	uint64_t FileID{0};
	auto EC = sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID);
	if (EC) {
	// The inode ID could not be determined, so create a hash value
	// the current file name and use that as an ID.
	FileID = hash_value(FileIDInfo);
	}
	else {
	FileID = ID.getFile();
	}
	uint64_t FileID = [&]() {
	sys::fs::UniqueID ID;
	if (!sys::fs::getUniqueID(std::get<0>(FileIDInfo), ID))
	return ID.getFile();
	return hash_value(FileIDInfo);
	}();

You probably only want to hash the file name, not the line number: hash_value(std::get<0>(FileIDInfo)).
Consider normalizing the file path before hashing: sys::fs::real_path.

[Meinersbur]

LGTM, but some style questions not addressed.

[Meinersbur]

Is there reason you do not want to normalize the filename for hashing?

[mjklemm]

@kparzysz @jdoerfert Ping :-)

[kparzysz]

LGTM

[Meinersbur]

Is there reason you do not want to normalize the filename for hashing?

I am still curious. You will get different IDs for somedir/../file.cpp and file.cpp (and File.cpp on case-insesitive file systems)