Fixes for builds that require strict X/Open and POSIX compatiblity

[daltenty]

Summary

• use alternative to MAP_ANONYMOUS for allocating mapped memory if it isn't available
• Use strtok_r instead of strsep as part of getting program path
• Don't try to find the width of a terminal using "struct winsize" and TIOCGWINSZ on POSIX builds. These aren't defined under POSIX (even though some platforms make them available when they shouldn't), so just check if we are doing a X/Open or POSIX compliant build first.

[hubert.reinterpretcast]

These are defined under POSIX

Do you mean that these aren't defined under POSIX?

llvm/lib/Support/Unix/Memory.inc

94

<ins>On</ins><del>on</del> platforms that have it<ins>,</ins> we can use MAP_ANONYMOUS to get a memory<ins>-</ins>mapped page without file backing, but we need a fallback <del>by</del><ins>of</ins> opening /dev/zero for strictly POSIX platforms instead<ins>.</ins>

97

I would rather have fd assigned separately on each branch of the #if than to have the rest of the statement inside conditional inclusion blocks.

102

Add a space after ==.

112

To be consistent with the ifdef, this should be #elif defined(MAP_ANON).

135

The file descriptor returned by open and stored in fd is not closed on this path.

llvm/lib/Support/Unix/Path.inc

157

Once assigned, s always has the same value as pv. Do we need s?

161

Add spaces after the commas in the argument list.

167

Add spaces after the commas in the argument list.
Add space after while (or at least that is what clang-format does for me).

[MaskRay]

llvm/lib/Support/Unix/Memory.inc

110

I think you can just keep MAP_ANON and remove MAP_ANONYMOUS. Though marked as deprecated in Linux manpages, it is most widely used. It is also used by compiler-rt.

[MaskRay]

llvm/lib/Support/Unix/Memory.inc

110

You may need to rebase. I've deleted MAP_ANONYMOUS.

[daltenty]

• Address formatting comments
• Revert to using MAP_ANON
• Close the file descriptor on all return paths in Memory::allocateMappedMemory
• Cleanup the pointers we keep in getprogpath

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Memory.inc

111

I think using MMFlags |= MAP_ANON; will help readability/maintainability.

145

If it's okay to close the file here, then it is okay to close the file right after the call to mmap before checking whether mmap failed. We would only need to add one call to close (instead of three).

llvm/lib/Support/Unix/Path.inc

158

If we're going to move where the pv, etc. is declared, then we might as well move the declaration of s to this line...

161

... and declare t in the if:

if (char *t = strtok_r(s, ":", &state)) {

[daltenty]

llvm/lib/Support/Unix/Memory.inc

145

If we close right after mmap, that would technically render the value of errno undefined or clobber it, and we may need to inspect it if the mmap failed (and according to spec we should only manipulate it if mmap failed)

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Path.inc

158

Minor nit: Add a space before the =.

161

Does this compile?

[daltenty]

Fix style

[daltenty]

• Fix loop on strtok_r

[daltenty]

llvm/lib/Support/Unix/Path.inc

161

Yes it does. Can you clarify?

[hubert.reinterpretcast]

LGTM.

llvm/lib/Support/Unix/Memory.inc

 Memory::allocateMappedMemory(size_t NumBytes,
                              const MemoryBlock *const NearBlock,
                              unsigned PFlags,
                              std::error_code &EC) {
   EC = std::error_code();
   if (NumBytes == 0)
     return MemoryBlock();
 
-  int fd = -1;
+  // on platforms that have it we can use MAP_ANONYMOUS to get a memory mapped
+  // page without file backing, but we need a fallback by opening /dev/zero
+  // for strictly POSIX platforms instead
+  int fd =
+#if defined(MAP_ANONYMOUS) || defined(MAP_ANON)
+   -1;
+#else
+  open("/dev/zero", O_RDWR);
+  if (fd ==-1) {
+    EC = std::error_code(errno, std::generic_category());
+    return MemoryBlock();
+  }
+#endif
+  
 
-  int MMFlags = MAP_PRIVATE |
+  int MMFlags = MAP_PRIVATE 
 #ifdef MAP_ANONYMOUS
-  MAP_ANONYMOUS
-#else
-  MAP_ANON
+  | MAP_ANONYMOUS
+#elif MAP_ANON
+  | MAP_ANON
 #endif
   ; // Ends statement above
 
   int Protect = getPosixProtectionFlags(PFlags);
 
 #if defined(__NetBSD__) && defined(PROT_MPROTECT)
   Protect |= PROT_MPROTECT(PROT_READ | PROT_WRITE | PROT_EXEC);
 #endif
 
   // Use any near hint and the page size to set a page-aligned starting address
   uintptr_t Start = NearBlock ? reinterpret_cast<uintptr_t>(NearBlock->base()) +
                                       NearBlock->size() : 0;
   static const size_t PageSize = Process::getPageSize();
   if (Start && Start % PageSize)
     Start += PageSize - Start % PageSize;
 
   // FIXME: Handle huge page requests (MF_HUGE_HINT).
   void *Addr = ::mmap(reinterpret_cast<void *>(Start), NumBytes, Protect,
                       MMFlags, fd, 0);
   if (Addr == MAP_FAILED) {
     if (NearBlock) //Try again without a near hint
       return allocateMappedMemory(NumBytes, nullptr, PFlags, EC);
 
     EC = std::error_code(errno, std::generic_category());
+#if !defined(MAP_ANONYMOUS) && !defined(MAP_ANON) 
+    close(fd);
+#endif 
     return MemoryBlock();
   }
 
+#if !defined(MAP_ANONYMOUS) && !defined(MAP_ANON) 
+  close(fd);
+#endif 
+
+
   MemoryBlock Result;
   Result.Address = Addr;
   Result.Size = NumBytes;
   Result.Flags = PFlags;
 
   // Rely on protectMappedMemory to invalidate instruction cache.
   if (PFlags & MF_EXEC) {
     EC = Memory::protectMappedMemory (Result, PFlags);

llvm/lib/Support/Unix/Path.inc

if (strchr(bin, '/')) {
     if (test_dir(ret, cwd, bin) == 0)
       return ret;
     return nullptr;
   }
 
   /* Third approach: $PATH */
   if ((pv = getenv("PATH")) == nullptr)
     return nullptr;
   s = pv = strdup(pv);
   if (!pv)
     return nullptr;
-  while ((t = strsep(&s, ":")) != nullptr) {
+  char *state;
+  if ((t = strtok_r(s,":",&state)) != nullptr) {
+    do {
       if (test_dir(ret, t, bin) == 0) {
         free(pv);
         return ret;
       }
+    } while((t = strtok_r(nullptr,":",&state)) != nullptr);
   }
   free(pv);
   return nullptr;
 }
 #endif // __FreeBSD__ || __NetBSD__ || __FreeBSD_kernel__
 
 /// GetMainExecutable - Return the path to the main executable, given the
 /// value of argv[0] from program startup.

llvm/lib/Support/Unix/Process.inc

static unsigned getColumns(int FileID) {
   if (const char *ColumnsStr = std::getenv("COLUMNS")) {
     int Columns = std::atoi(ColumnsStr);
     if (Columns > 0)
       return Columns;
   }
 
   unsigned Columns = 0;
 
-#if defined(HAVE_SYS_IOCTL_H) && defined(HAVE_TERMIOS_H)
+#if defined(HAVE_SYS_IOCTL_H) && defined(HAVE_TERMIOS_H) \
+  && !(defined(_XOPEN_SOURCE) || defined(_POSIX_C_SOURCE))
   // Try to determine the width of the terminal.
   struct winsize ws;
   if (ioctl(FileID, TIOCGWINSZ, &ws) == 0)
     Columns = ws.ws_col;
 #endif
 
   return Columns;
 }