Reland: [libc] implement ioctl #85890 #90317

changkhothuychung · 2024-04-27T07:05:17Z

Fix #85275

This reverts commit 3c2feab.

llvmbot · 2024-04-27T07:05:49Z

@llvm/pr-subscribers-libc

Author: Nhat Nguyen (changkhothuychung)

Changes

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

13 Files Affected:

(modified) libc/config/linux/aarch64/entrypoints.txt (+3)
(modified) libc/config/linux/riscv/entrypoints.txt (+3)
(modified) libc/config/linux/x86_64/entrypoints.txt (+3)
(modified) libc/spec/linux.td (+18)
(modified) libc/src/sys/CMakeLists.txt (+1)
(added) libc/src/sys/ioctl/CMakeLists.txt (+12)
(added) libc/src/sys/ioctl/ioctl.h (+17)
(added) libc/src/sys/ioctl/linux/CMakeLists.txt (+13)
(added) libc/src/sys/ioctl/linux/ioctl.cpp (+39)
(modified) libc/test/src/sys/CMakeLists.txt (+5-3)
(added) libc/test/src/sys/ioctl/CMakeLists.txt (+3)
(added) libc/test/src/sys/ioctl/linux/CMakeLists.txt (+13)
(added) libc/test/src/sys/ioctl/linux/ioctl_test.cpp (+27)

diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 78da7f0b334b1fe..b5f8454081c8984 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -204,6 +204,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     #libc.src.stdio.scanf
     #libc.src.stdio.fscanf
 
+    # sys/ioctl.h entrypoints
+    libc.src.sys.ioctl.ioctl
+
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
     libc.src.sys.mman.mmap
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 5aae4e246cfb3c2..da9f0a9fb5b506d 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -209,6 +209,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.scanf
     libc.src.stdio.fscanf
 
+    # sys/ioctl.h entrypoints
+    libc.src.sys.ioctl.ioctl
+
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
     libc.src.sys.mman.mmap
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 5b428e51aee6206..923946a75a24f59 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -221,6 +221,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     # https://github.com/llvm/llvm-project/issues/80060
     # libc.src.sys.epoll.epoll_pwait2
 
+    # sys/ioctl.h entrypoints
+    libc.src.sys.ioctl.ioctl
+
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
     libc.src.sys.mman.mmap
diff --git a/libc/spec/linux.td b/libc/spec/linux.td
index f91f55ddac78466..6e9b64f5a6b4aff 100644
--- a/libc/spec/linux.td
+++ b/libc/spec/linux.td
@@ -79,6 +79,24 @@ def Linux : StandardSpec<"Linux"> {
       []  // Functions
   >;
 
+  HeaderSpec SysIoctl = HeaderSpec<
+      "sys/ioctl.h",
+      [Macro<"MAP_ANONYMOUS">],
+      [], // Types
+      [], // Enumerations
+      [
+        FunctionSpec<
+            "ioctl",
+            RetValSpec<IntType>,
+            [
+              ArgSpec<IntType>,
+              ArgSpec<UnsignedLongType>,
+              ArgSpec<VoidPtr>,
+            ]
+        >,
+      ]  // Functions
+  >;
+
   HeaderSpec SysMMan = HeaderSpec<
       "sys/mman.h",
       [Macro<"MAP_ANONYMOUS">],
diff --git a/libc/src/sys/CMakeLists.txt b/libc/src/sys/CMakeLists.txt
index adc666b94202f7d..ac54df35284a7b8 100644
--- a/libc/src/sys/CMakeLists.txt
+++ b/libc/src/sys/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_subdirectory(auxv)
 add_subdirectory(epoll)
+add_subdirectory(ioctl)
 add_subdirectory(mman)
 add_subdirectory(random)
 add_subdirectory(resource)
diff --git a/libc/src/sys/ioctl/CMakeLists.txt b/libc/src/sys/ioctl/CMakeLists.txt
new file mode 100644
index 000000000000000..4b50c278c7871a8
--- /dev/null
+++ b/libc/src/sys/ioctl/CMakeLists.txt
@@ -0,0 +1,12 @@
+if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+  add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+endif()
+
+add_entrypoint_object(
+  ioctl
+  ALIAS
+  DEPENDS
+    .${LIBC_TARGET_OS}.ioctl
+)
+
+
diff --git a/libc/src/sys/ioctl/ioctl.h b/libc/src/sys/ioctl/ioctl.h
new file mode 100644
index 000000000000000..8365678276a4286
--- /dev/null
+++ b/libc/src/sys/ioctl/ioctl.h
@@ -0,0 +1,17 @@
+//===-- Implementation header for mmap function -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SYS_IOCTL_IOCTL_H
+#define LLVM_LIBC_SRC_SYS_IOCTL_IOCTL_H
+namespace LIBC_NAMESPACE {
+
+int ioctl(int fd, unsigned long request, ...);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_SYS_IOCTL_IOCTL_H
diff --git a/libc/src/sys/ioctl/linux/CMakeLists.txt b/libc/src/sys/ioctl/linux/CMakeLists.txt
new file mode 100644
index 000000000000000..8a23505d4e9d19e
--- /dev/null
+++ b/libc/src/sys/ioctl/linux/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_entrypoint_object(
+  ioctl
+  SRCS
+    ioctl.cpp
+  HDRS
+    ../ioctl.h
+  DEPENDS
+    libc.include.sys_ioctl
+    libc.include.sys_syscall
+    libc.src.__support.OSUtil.osutil
+    libc.src.errno.errno
+)
+
diff --git a/libc/src/sys/ioctl/linux/ioctl.cpp b/libc/src/sys/ioctl/linux/ioctl.cpp
new file mode 100644
index 000000000000000..c743cf7a9dd5969
--- /dev/null
+++ b/libc/src/sys/ioctl/linux/ioctl.cpp
@@ -0,0 +1,39 @@
+//===---------- Linux implementation of the POSIX ioctl function --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/sys/ioctl/ioctl.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+#include "src/errno/libc_errno.h"
+#include <stdarg.h>
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE {
+
+// This function is currently linux only. It has to be refactored suitably if
+// madvise is to be supported on non-linux operating systems also.
+LLVM_LIBC_FUNCTION(int, ioctl, (int fd, unsigned long request, ...)) {
+  va_list ptr_to_memory;
+  va_start(ptr_to_memory, request);
+  va_arg(ptr_to_memory, void *);
+  int ret =
+      LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, request, ptr_to_memory);
+  va_end(ptr_to_memory);
+
+  // A negative return value indicates an error with the magnitude of the
+  // value being the error code.
+  if (ret < 0) {
+    libc_errno = -ret;
+    return -1;
+  }
+
+  return 0;
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/sys/CMakeLists.txt b/libc/test/src/sys/CMakeLists.txt
index dc0aa8bf7b75dc1..b58644e60f57872 100644
--- a/libc/test/src/sys/CMakeLists.txt
+++ b/libc/test/src/sys/CMakeLists.txt
@@ -1,4 +1,8 @@
+add_subdirectory(auxv)
+add_subdirectory(epoll)
+add_subdirectory(ioctl)
 add_subdirectory(mman)
+add_subdirectory(prctl)
 add_subdirectory(random)
 add_subdirectory(resource)
 add_subdirectory(select)
@@ -8,6 +12,4 @@ add_subdirectory(stat)
 add_subdirectory(statvfs)
 add_subdirectory(utsname)
 add_subdirectory(wait)
-add_subdirectory(prctl)
-add_subdirectory(auxv)
-add_subdirectory(epoll)
+
diff --git a/libc/test/src/sys/ioctl/CMakeLists.txt b/libc/test/src/sys/ioctl/CMakeLists.txt
new file mode 100644
index 000000000000000..b4bbe81c92ff2eb
--- /dev/null
+++ b/libc/test/src/sys/ioctl/CMakeLists.txt
@@ -0,0 +1,3 @@
+if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
+  add_subdirectory(${LIBC_TARGET_OS})
+endif()
diff --git a/libc/test/src/sys/ioctl/linux/CMakeLists.txt b/libc/test/src/sys/ioctl/linux/CMakeLists.txt
new file mode 100644
index 000000000000000..aa1bffec16bd672
--- /dev/null
+++ b/libc/test/src/sys/ioctl/linux/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_custom_target(libc_sys_ioctl_unittests)
+
+add_libc_unittest(
+  ioctl_test
+  SUITE
+    libc_sys_ioctl_unittests
+  SRCS
+    ioctl_test.cpp
+  DEPENDS
+    libc.src.sys.ioctl.ioctl
+    libc.src.errno.errno
+)
+
diff --git a/libc/test/src/sys/ioctl/linux/ioctl_test.cpp b/libc/test/src/sys/ioctl/linux/ioctl_test.cpp
new file mode 100644
index 000000000000000..4fc76d1b3c1842c
--- /dev/null
+++ b/libc/test/src/sys/ioctl/linux/ioctl_test.cpp
@@ -0,0 +1,27 @@
+//===-- Unittests for ioctl -----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/errno/libc_errno.h"
+#include "src/sys/ioctl/ioctl.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/LibcTest.h"
+#include "test/UnitTest/Test.h"
+
+#include <sys/syscall.h>
+#include <unistd.h>
+
+using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
+using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+
+TEST(LlvmLibcIoctlTest, InvalidFileDescriptor) {
+  int fd = 10;
+  unsigned long request = 10;
+  int res = LIBC_NAMESPACE::ioctl(fd, request, NULL);
+  EXPECT_THAT(res, Fails(EBADF, -1));
+}

changkhothuychung · 2024-04-27T15:19:31Z

@nickdesaulniers nick
can u take a look when you have time? thanks.

michaelrj-google

Nick's busy for a couple weeks so I'll be reviewing this patch.

michaelrj-google · 2024-05-06T22:36:59Z

libc/src/sys/ioctl/linux/ioctl.cpp

+namespace LIBC_NAMESPACE {
+
+// This function is currently linux only. It has to be refactored suitably if
+// madvise is to be supported on non-linux operating systems also.


nit: update comment to refer to the correct function instead of madvise.

michaelrj-google · 2024-05-06T22:37:41Z

libc/test/src/sys/ioctl/linux/ioctl_test.cpp

+TEST(LlvmLibcIoctlTest, InvalidFileDescriptor) {
+  int fd = 10;
+  unsigned long request = 10;
+  int res = LIBC_NAMESPACE::ioctl(fd, request, NULL);


please add a test that succeeds as well.

@michaelrj-google do you have an idea what test can I use to always guarantee success ? From the other PR @nickdesaulniers nick
told me I dont need to test for this syscall because it is a linux syscall, and the current test file is fine. I tried a test that I expect to succeeds in the PR but that seems to fail in the CI.

Perhaps FS_IOC_GETFLAGS.
https://man7.org/linux/man-pages/man2/ioctl_iflags.2.html

That is used as the example for ioctl in Michael Kerrisk's The Linux Programming Interface page 308.

nickdesaulniers · 2024-05-20T18:38:10Z

libc/test/src/sys/ioctl/linux/ioctl_test.cpp

+  int fd = open("/dev/null", O_RDWR);
+  int data;
+  int res = LIBC_NAMESPACE::ioctl(fd, FIONREAD, &data);
+  EXPECT_THAT(res, Succeeds());


From the presubmit build failure:

ioctl_test.cpp:35: FAILURE Failed to match res against Succeeds(). Expected return value to be equal to 0 but got -1. Expected errno to be equal to "Success" but got "Inappropriate ioctl for device".

I applied FS_IOC_GETFLAGS but it seems the same error is still there

it looks like the error In appropriate ioctl for device is for the case the test wants to connect to terminal but there is no terminal on the CI.

This is perhaps related to the usage of /dev/null as the input file. Perhaps we need to create a new file and run the test on that.

$ lsattr /dev/null lsattr: Operation not supported While reading flags on /dev/null

See how libc/test/src/stdio/ tests create temp files.

looks like it works now

make sure to close the file

@nickdesaulniers nick
do u know why I am getting error undefined symbol to libc_namspace::open() and close()?

Is that a linkage failure? Then that would be a missing dependency in cmake for the test.

…m-project into ioctl_impl

nickdesaulniers · 2024-06-18T22:43:53Z

Is this ready for re-review, or?

changkhothuychung · 2024-06-23T19:21:58Z

Is this ready for re-review, or?

Yes, this is ready for review now! Thanks.

changkhothuychung · 2024-07-29T04:13:40Z

@nickdesaulniers Hi, any update on this? Also can you help me confirm with the full-build mode? My laptop (mac m1) has issues with full-build mode, thanks!

nickdesaulniers · 2024-10-10T18:12:25Z

libc/test/src/sys/ioctl/linux/ioctl_test.cpp

+  LIBC_NAMESPACE::libc_errno = 0;
+  constexpr const char *FILENAME = "ioctl.test";
+  auto TEST_FILE = libc_make_test_file_path(FILENAME);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);


/android0/llvm-project/libc/test/src/sys/ioctl/linux/ioctl_test.cpp:33:64: error: use of undeclared identifier 'S_IRWXU' 33 | int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU); | ^

need to include sys/stat.h, though perhaps we should have a proxy header (akin to libc/hdr/sys_epoll_macros.h) for that.

michaelrj-google · 2025-04-04T22:27:12Z

@changkhothuychung and @SchrodingerZhu, are you planning to finish this PR?

changkhothuychung added 2 commits April 27, 2024 01:33

Revert "Revert "[libc] implement ioctl" (llvm#88226)"

05a886d

This reverts commit 3c2feab.

reland ioctl

e1a93de

llvmbot added the libc label Apr 27, 2024

lntue requested review from SchrodingerZhu and michaelrj-google May 6, 2024 13:22

michaelrj-google reviewed May 6, 2024

View reviewed changes

changkhothuychung added 5 commits May 11, 2024 10:32

add success test

236873d

add lib

4f1c749

syntax fix

1578645

try a new device

e66e13b

missing header

de63af0

nickdesaulniers reviewed May 20, 2024

View reviewed changes

changkhothuychung added 4 commits May 21, 2024 19:23

update test file

c02aab6

header update

8cccec9

missing header

5100e86

chang file

84147f8

changkhothuychung requested a review from nickdesaulniers May 23, 2024 04:10

changkhothuychung added 9 commits June 2, 2024 15:22

close the file

49f745a

some fix

805266f

fix

40b9b29

more fix

d6ce110

close file

7d8d9d7

use internal function

4e07b1c

Merge branch 'main' into ioctl_impl

7b76aeb

add to lib

b784f43

Merge branch 'ioctl_impl' of https://github.com/changkhothuychung/llv…

0bb8dd7

…m-project into ioctl_impl

fix test

fd3bad5

changkhothuychung added 4 commits June 23, 2024 14:39

fix test

0b8f0db

fix text

e5e65f8

remove headers

7b55dee

fix header

82e9533

lntue mentioned this pull request Jul 3, 2024

[libc] Add basic ioctl function #97509

Closed

jhuber6 changed the title ~~Reland: [libc] implement ioctl ([libc] implement ioctl #85890)~~ Reland: [libc] implement ioctl #85890 Jul 3, 2024

izaakschroeder mentioned this pull request Jul 3, 2024

[libc] build clang #97191

Open

39 tasks

nickdesaulniers reviewed Oct 10, 2024

View reviewed changes

cowtoolz mentioned this pull request May 25, 2025

[libc] add ioctl #141393

Open

Reland: [libc] implement ioctl #85890 #90317

Are you sure you want to change the base?

Reland: [libc] implement ioctl #85890 #90317

Uh oh!

Conversation

changkhothuychung commented Apr 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 27, 2024

Uh oh!

changkhothuychung commented Apr 27, 2024

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickdesaulniers May 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickdesaulniers commented Jun 18, 2024

Uh oh!

changkhothuychung commented Jun 23, 2024

Uh oh!

changkhothuychung commented Jul 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelrj-google commented Apr 4, 2025

Uh oh!

Uh oh!

changkhothuychung commented Apr 27, 2024 •

edited

Loading

nickdesaulniers May 20, 2024 •

edited

Loading