[Support] Treat truncation of fullpath as error

[Hahnfeld]

Summary

If the concatenation of arguments dir and bin has at least PATH_MAX
characters the call to snprintf will truncate. The result will usually
not exist, but if it does it's actually incorrect to return that the
path exists.
(Motivated by GCC compiler warning about format truncation.)

[efriedma]

llvm/lib/Support/Unix/Path.inc

114 ↗

(On Diff #188938)

The use of PATH_MAX here seems dubious; in practice, many targets support paths longer than PATH_MAX. Probably should just use a raw_string_ostream or something like that instead of snprintf to a fixed buffer.

[zturner]

llvm/lib/Support/Unix/Path.inc

114 ↗

(On Diff #188938)

Even better, llvm::sys::path::join(). I'm not sure why all of these global functions operate on raw pointers instead of LLVM's string ADTs.

[Hahnfeld]

llvm/lib/Support/Unix/Path.inc

114 ↗

(On Diff #188938)

I played a bit on my Arch Linux system: It's indeed possible to create paths that are longer than PATH_MAX = 4096. But I can't get the realpath nor even cd into there, and I'd guess that many more tools are broken.

In that case I think there's no difference between fixed buffers and the string ADTs, except that the latter are probably slower.

[Hahnfeld]

Ping. As I said I think the current approach with a fixed-size buffer makes sense because anything larger likely won't work anyhow.

[efriedma]

LGTM

llvm/lib/Support/Unix/Path.inc

114 ↗

(On Diff #188938)

I guess this is okay given the way the code is currently written.

It should probably be rewritten eventually to work in a more consistent way on systems which don't have a PATH_MAX, like Hurd, but that shouldn't block this patch, I think.

llvm/trunk/lib/Support/Unix/Path.inc

#if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) ||     \
     defined(__minix) || defined(__FreeBSD_kernel__) || defined(__linux__) ||   \
     defined(__CYGWIN__) || defined(__DragonFly__) || defined(_AIX) || defined(__GNU__)
 static int
 test_dir(char ret[PATH_MAX], const char *dir, const char *bin)
 {
   struct stat sb;
   char fullpath[PATH_MAX];
 
-  snprintf(fullpath, PATH_MAX, "%s/%s", dir, bin);
+  int chars = snprintf(fullpath, PATH_MAX, "%s/%s", dir, bin);
+  // We cannot write PATH_MAX characters because the string will be terminated
+  // with a null character. Fail if truncation happened.
+  if (chars >= PATH_MAX)
+    return 1;
   if (!realpath(fullpath, ret))
     return 1;
   if (stat(fullpath, &sb) != 0)
     return 1;
 
   return 0;
 }