https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/143514
On unix systems, we were trying to determine the terminal width using the `COULMNS` environment variable. Unfortunately, `COLUMNS` is not a proper environment variable, but rather a shell variable and a such is not exported to actual processes (even though it *is* available in e.g. shell scripts) on some systems, including mine.
We were previously using the termios API for this, but we stopped doing that, apparently because it wasn’t fully cross-platform. As for what systems that are, I’m not sure. I’d expect it to be available on every system that provides both `<sys/ioctl.h>` and `<termios.h>`, but if that isn’t the case, then we can also just add e.g. `&& defined(__linux__)` because that’d fix this at least for some systems.
This essentially reverts a3eb3d3d92d037fe3c9deaad87f6fc42fe9ea766 and parts of https://reviews.llvm.org/D61326.
For more information, see #139499.
Fixes #139499.
>From 223e57e6b3b1f053b369088c032d05443e528e08 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 9 Jun 2025 03:26:14 +0200
Subject: [PATCH] [LLVM] [Support] Query the terminal width using the termios
API
This essentially reverts a3eb3d3d92d037fe3c9deaad87f6fc42fe9ea766.
For more information, see #139499.
Fixes #139499.
---
llvm/cmake/config-ix.cmake | 10 +++++++++
llvm/include/llvm/Config/config.h.cmake | 6 ++++++
llvm/lib/Support/Unix/Process.inc | 28 +++++++++++++++++++------
3 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index 687f5077cbfd2..319b0c0267005 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -24,6 +24,8 @@ if (ANDROID OR CYGWIN OR CMAKE_SYSTEM_NAME MATCHES "AIX|DragonFly|FreeBSD|Haiku|
set(HAVE_SYS_MMAN_H 1)
set(HAVE_SYSEXITS_H 1)
set(HAVE_UNISTD_H 1)
+ set(HAVE_SYS_IOCTL_H 1)
+ set(HAVE_TERMIOS_H 1)
elseif (APPLE)
set(HAVE_MACH_MACH_H 1)
set(HAVE_MALLOC_MALLOC_H 1)
@@ -31,6 +33,8 @@ elseif (APPLE)
set(HAVE_SYS_MMAN_H 1)
set(HAVE_SYSEXITS_H 1)
set(HAVE_UNISTD_H 1)
+ set(HAVE_SYS_IOCTL_H 1)
+ set(HAVE_TERMIOS_H 1)
elseif (PURE_WINDOWS)
set(HAVE_MACH_MACH_H 0)
set(HAVE_MALLOC_MALLOC_H 0)
@@ -38,6 +42,8 @@ elseif (PURE_WINDOWS)
set(HAVE_SYS_MMAN_H 0)
set(HAVE_SYSEXITS_H 0)
set(HAVE_UNISTD_H 0)
+ set(HAVE_SYS_IOCTL_H 0)
+ set(HAVE_TERMIOS_H 0)
elseif (ZOS)
# Confirmed in
# https://github.com/llvm/llvm-project/pull/104706#issuecomment-2297109613
@@ -47,6 +53,8 @@ elseif (ZOS)
set(HAVE_SYS_MMAN_H 1)
set(HAVE_SYSEXITS_H 0)
set(HAVE_UNISTD_H 1)
+ set(HAVE_SYS_IOCTL_H 1)
+ set(HAVE_TERMIOS_H 1)
else()
# Other platforms that we don't promise support for.
check_include_file(mach/mach.h HAVE_MACH_MACH_H)
@@ -55,6 +63,8 @@ else()
check_include_file(sys/mman.h HAVE_SYS_MMAN_H)
check_include_file(sysexits.h HAVE_SYSEXITS_H)
check_include_file(unistd.h HAVE_UNISTD_H)
+ check_include_file(sys/ioctl.h HAVE_SYS_IOCTL_H)
+ check_include_file(termios.h HAVE_TERMIOS_H)
endif()
if( UNIX AND NOT (APPLE OR BEOS OR HAIKU) )
diff --git a/llvm/include/llvm/Config/config.h.cmake b/llvm/include/llvm/Config/config.h.cmake
index 06d4756397911..3f0fa843974d2 100644
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -164,6 +164,12 @@
/* Define to 1 if you have the <sys/mman.h> header file. */
#cmakedefine HAVE_SYS_MMAN_H ${HAVE_SYS_MMAN_H}
+/* Define to 1 if you have the <sys/ioctl.h> header file. */
+#cmakedefine HAVE_SYS_IOCTL_H ${HAVE_SYS_IOCTL_H}
+
+/* Define to 1 if you have the <termios.h> header file. */
+#cmakedefine HAVE_TERMIOS_H ${HAVE_TERMIOS_H}
+
/* Define to 1 if stat struct has st_mtimespec member .*/
#cmakedefine HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC ${HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC}
diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc
index b5c3719f57963..c4ffa5b2749bd 100644
--- a/llvm/lib/Support/Unix/Process.inc
+++ b/llvm/lib/Support/Unix/Process.inc
@@ -34,6 +34,12 @@
#ifdef HAVE_GETAUXVAL
#include <sys/auxv.h>
#endif
+#ifdef HAVE_SYS_IOCTL_H
+#include <sys/ioctl.h>
+#endif
+#ifdef HAVE_TERMIOS_H
+#include <termios.h>
+#endif
//===----------------------------------------------------------------------===//
//=== WARNING: Implementation here must contain only generic UNIX code that
@@ -304,7 +310,7 @@ bool Process::FileDescriptorIsDisplayed(int fd) {
#endif
}
-static unsigned getColumns() {
+static unsigned getColumns([[maybe_unused]] int FileID) {
// If COLUMNS is defined in the environment, wrap to that many columns.
if (const char *ColumnsStr = std::getenv("COLUMNS")) {
int Columns = std::atoi(ColumnsStr);
@@ -312,23 +318,33 @@ static unsigned getColumns() {
return Columns;
}
- // We used to call ioctl TIOCGWINSZ to determine the width. It is considered
- // unuseful.
- return 0;
+ // Unfortunately, COLUMNS is a shell variable, not a proper environment
+ // variable, so it is often not available; query the column count via
+ // the termios API instead if it isn't.
+ unsigned Columns = 0;
+
+#if defined(HAVE_SYS_IOCTL_H) && defined(HAVE_TERMIOS_H)
+ // Try to determine the width of the terminal.
+ struct winsize ws;
+ if (ioctl(FileID, TIOCGWINSZ, &ws) == 0)
+ Columns = ws.ws_col;
+#endif
+
+ return Columns;
}
unsigned Process::StandardOutColumns() {
if (!StandardOutIsDisplayed())
return 0;
- return getColumns();
+ return getColumns(0);
}
unsigned Process::StandardErrColumns() {
if (!StandardErrIsDisplayed())
return 0;
- return getColumns();
+ return getColumns(1);
}
static bool terminalHasColors() {