From 223e57e6b3b1f053b369088c032d05443e528e08 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Mon, 9 Jun 2025 03:26:14 +0200 Subject: [PATCH 1/5] [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 header file. */ #cmakedefine HAVE_SYS_MMAN_H ${HAVE_SYS_MMAN_H} +/* Define to 1 if you have the header file. */ +#cmakedefine HAVE_SYS_IOCTL_H ${HAVE_SYS_IOCTL_H} + +/* Define to 1 if you have the 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 #endif +#ifdef HAVE_SYS_IOCTL_H +#include +#endif +#ifdef HAVE_TERMIOS_H +#include +#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() { From 47d434ee1d9a6bbd4a8dfdb125fc6eef4d59815e Mon Sep 17 00:00:00 2001 From: Sirraide Date: Sat, 14 Jun 2025 14:41:23 +0200 Subject: [PATCH 2/5] Remove check for termios.h --- llvm/cmake/config-ix.cmake | 5 ----- llvm/include/llvm/Config/config.h.cmake | 3 --- llvm/lib/Support/Unix/Process.inc | 5 +---- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake index 319b0c0267005..3f1fc4e197216 100644 --- a/llvm/cmake/config-ix.cmake +++ b/llvm/cmake/config-ix.cmake @@ -25,7 +25,6 @@ if (ANDROID OR CYGWIN OR CMAKE_SYSTEM_NAME MATCHES "AIX|DragonFly|FreeBSD|Haiku| 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) @@ -34,7 +33,6 @@ elseif (APPLE) 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) @@ -43,7 +41,6 @@ elseif (PURE_WINDOWS) 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 @@ -54,7 +51,6 @@ elseif (ZOS) 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) @@ -64,7 +60,6 @@ else() 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 3f0fa843974d2..ce83de8e4cba9 100644 --- a/llvm/include/llvm/Config/config.h.cmake +++ b/llvm/include/llvm/Config/config.h.cmake @@ -167,9 +167,6 @@ /* Define to 1 if you have the header file. */ #cmakedefine HAVE_SYS_IOCTL_H ${HAVE_SYS_IOCTL_H} -/* Define to 1 if you have the 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 c4ffa5b2749bd..79ea09c71a2f8 100644 --- a/llvm/lib/Support/Unix/Process.inc +++ b/llvm/lib/Support/Unix/Process.inc @@ -37,9 +37,6 @@ #ifdef HAVE_SYS_IOCTL_H #include #endif -#ifdef HAVE_TERMIOS_H -#include -#endif //===----------------------------------------------------------------------===// //=== WARNING: Implementation here must contain only generic UNIX code that @@ -323,7 +320,7 @@ static unsigned getColumns([[maybe_unused]] int FileID) { // the termios API instead if it isn't. unsigned Columns = 0; -#if defined(HAVE_SYS_IOCTL_H) && defined(HAVE_TERMIOS_H) +#ifdef HAVE_SYS_IOCTL_H // Try to determine the width of the terminal. struct winsize ws; if (ioctl(FileID, TIOCGWINSZ, &ws) == 0) From 526c4189b351cd96c5c97e2d08ff3bc3f2edf267 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Sat, 14 Jun 2025 14:43:02 +0200 Subject: [PATCH 3/5] Adjust comment --- llvm/lib/Support/Unix/Process.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc index 79ea09c71a2f8..31fad7bb7e2c5 100644 --- a/llvm/lib/Support/Unix/Process.inc +++ b/llvm/lib/Support/Unix/Process.inc @@ -317,7 +317,7 @@ static unsigned getColumns([[maybe_unused]] int FileID) { // 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. + // ioctl() instead if it isn't. unsigned Columns = 0; #ifdef HAVE_SYS_IOCTL_H From bdbae6947dcfc8ef0335a858597322459316f017 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Sun, 15 Jun 2025 14:06:17 +0200 Subject: [PATCH 4/5] Address review comments --- llvm/lib/Support/Unix/Process.inc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc index 31fad7bb7e2c5..db735b7484ad8 100644 --- a/llvm/lib/Support/Unix/Process.inc +++ b/llvm/lib/Support/Unix/Process.inc @@ -307,21 +307,20 @@ bool Process::FileDescriptorIsDisplayed(int fd) { #endif } -static unsigned getColumns([[maybe_unused]] int FileID) { +static unsigned getColumns(int FileID) { // If COLUMNS is defined in the environment, wrap to that many columns. + // This matches GCC. if (const char *ColumnsStr = std::getenv("COLUMNS")) { int Columns = std::atoi(ColumnsStr); if (Columns > 0) return Columns; } - // Unfortunately, COLUMNS is a shell variable, not a proper environment - // variable, so it is often not available; query the column count via - // ioctl() instead if it isn't. + // Some shells do not export COLUMNS; query the column count via ioctl() + // instead if it isn't available. unsigned Columns = 0; #ifdef HAVE_SYS_IOCTL_H - // Try to determine the width of the terminal. struct winsize ws; if (ioctl(FileID, TIOCGWINSZ, &ws) == 0) Columns = ws.ws_col; From 04f9989bf7b2d423f06e2dff48e791474dfacb1c Mon Sep 17 00:00:00 2001 From: Sirraide Date: Mon, 16 Jun 2025 17:42:03 +0200 Subject: [PATCH 5/5] Fix merge error --- llvm/cmake/config-ix.cmake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake index a55d04fda7708..0fcd73e752311 100644 --- a/llvm/cmake/config-ix.cmake +++ b/llvm/cmake/config-ix.cmake @@ -27,9 +27,8 @@ elseif (APPLE) set(HAVE_SYS_MMAN_H 1) set(HAVE_SYSEXITS_H 1) set(HAVE_UNISTD_H 1) -elseif (WIN32) set(HAVE_SYS_IOCTL_H 1) -elseif (PURE_WINDOWS) +elseif (WIN32) set(HAVE_MACH_MACH_H 0) set(HAVE_MALLOC_MALLOC_H 0) set(HAVE_PTHREAD_H 0)