[libc] Implement getopt

[abrachet]

I wanted to get this out fairly early because there are quite a few things that I wanted to get your input on hdr-gen and policy questions etc

libc/spec/posix.td

590

This is a bit of a sad situation. Do you have any thoughts on this, @sivachandra

596

I've just remembered I need to add ObjectSpec's for the global objects. I'll do that in the next round

libc/src/unistd/getopt.cpp

108

How should we be pulling in stdio bits? It seems like we've been trying to make an effort to call other __llvm_libc:: entrypoints from each other. Though fprintf is sufficiently big that I think we would have to just do this. Alternatively we could just use fprintf and that could be either the system fprintf or happen to be llvm-libc's if thats linked in. I'm not sure what our policy is here.

libc/src/unistd/getopt.h

17

Currently the only prior art for extern symbols like this is std{in,out,err}. Though I have some reservations on how those are implemented/tested. Functions being in the __llvm_libc namespace for testing is really useful and I think we should be able to do the same for global objects with aliases just like for functions to ensure hermeticity. I wanted to get your opinion on if you had tried this before.

[sivachandra]

libc/spec/posix.td

590

The trick would be to declare an internal type like:

typedef char *const __getopt_argv_type[];

In the tablegen files, define a NamedType for it and add a type header like this: https://github.com/llvm/llvm-project/blob/main/libc/include/llvm-libc-types/__atexithandler_t.h

596

SGTM

libc/src/unistd/getopt.cpp

108

Can we extend GetoptState [1] with an errstream field which:

1. Defaults to __llvm_libc::stderr.
2. Tests can override with their own custom File specialization, say for example a StrFile, which writes to a string buffer.

[1] - May be we should give GetoptState a better name.

libc/src/unistd/getopt.h

17

I think the examples of stdin and stdout are inappropriate for future global state variables. I want to suggest the following:

1. Do not declare the getopt state variables here. Instead, provide a method by name set_getopt_state which is declared as:

void set_getopt_state(char **optarg, int *optind, int *optopt, int optpos, int opterr);

2. In the .cpp file do this:

namespace impl {

extern "C" char *optarg;
extern "C" int optind = 1;
extern "C" int optopt;
extern "C" int opterr;
extern "C" int optpos = 0;

}

struct GetoptState {
  char **optarg;
  int *optind;
  int *optopt;
  unsigned *optpos;

  int opterr;
};
// The default state
static GetoptState state{&impl::optarg, &impl::optind, &impl::optopt, &impl::optpos, impl::opterr};

void set_getopt_state(char **optarg, int *optind, int *optopt, int *optpos, int opterr) {
  state.optarg = optarg;
  state.optind = optind;
  state.optopt = optopt;
  state.optpos = optpos;
  state.opterr = opterr;
}

The high level goal is to let tests specify where the state of getopt is to be stored by calling set_getopt_state. We can choose to exclude that function under LLVM_LIBC_PUBLIC_PACKAGING.

[abrachet]

libc/spec/posix.td

590

Ah that's right. I forgot about that. Thanks

libc/src/unistd/getopt.cpp

108

I've named it GetoptContext, which might be a better name.

I've used just stderr instead of __llvm_libc::stderr and same with fprintf. We were hoping to use this on Windows, actually, and pulling in llvm-libc's stdio isn't ready yet in that context. Likewise we are likely to pull this into Fuchsia's libc before stdio too. It would be helpful if we could do with stdio here like we do with malloc elsewhere. Is that okay?

[sivachandra]

Still a few comments about the high-level structuring. Once these are sorted out, I will do a pass over the parsing logic.

libc/src/unistd/getopt.cpp

108

This problem is likely not unique to getopt. WDYT about a build flag LIBC_USE_EXTERNAL_STDIO which gets converted to compile option -DUSE_EXTERNAL_STDIO. In the source code, when USE_EXTERNAL_STDIO is defined, we use public names to refer to symbols. By default, we use __llvm_libc::<name>.

libc/src/unistd/getopt.h

17

Do we still need the extern declarations?

libc/test/src/unistd/getopt_test.cpp

19

This can go into the implementation header. Any reason not to put it there?

60

This is going to make a lot of things a little messy - I am fine with your code here, but I am just thinking about how this is going to work in the FULL_BUILD mode.

May be tests can also look at the LIBC_USE_EXTERNAL_STDIO flag and conditionally add the appropriate DEPENDS. In the source code:

#ifdef USE_EXTERNAL_STDIO
define FOPENCOOKIE fopencookie
...
#else
#include "src/stdio/fopencookie.h"
define FOPENCOOKIE __llvm_libc::fopencookie
...
#endif

68

reset_errstream?

69

get_error_msg?

[abrachet]

libc/src/unistd/getopt.cpp

108

For now, I'll just use __llvm_libc::* and deal with using external stdio later.

libc/src/unistd/getopt.h

17

Nope, thanks.

libc/test/src/unistd/getopt_test.cpp

60

Per other comment, I've just gone and used __llvm_libc::fopencookie

[sivachandra]

LGTM with nits.

libc/src/unistd/getopt.cpp

18

Add a comment here saying that this implementation currently does not support GNU extensions.

41

report_error

149

Subjective comment - ISTM that I could have followed this logic readily if it was arranged this way:

++ctx.optind;
if (ctx.optind >= argc ...) {
}
ctx.optarg.get() = argv[ctx.optind];

libc/test/src/unistd/getopt_test.cpp

116

We actually follow this style: int('?') Better may be, like you have added the _c suffix, add _i suffix.

[abrachet]

Going to let the premerge run before pushing, because it wasn't working without full build before

[abrachet]

libc/src/unistd/getopt.cpp

149

I've added a comment to explain why I prefer to keep it as is, because optind should stay at the previous value upon return

[abrachet]

Rebase to let presubmit run

[abrachet]

Looks like libc-api-test is failing. I'll look at it tomorrow

libc/config/linux/aarch64/entrypoints.txt

list(APPEND TARGET_LIBC_ENTRYPOINTS
     libc.src.time.gmtime_r
     libc.src.time.mktime
     libc.src.time.nanosleep
     libc.src.time.time
 
     # unistd.h entrypoints
     libc.src.unistd.environ
     libc.src.unistd.execv
+    libc.src.unistd.getopt
+    libc.src.unistd.optarg
+    libc.src.unistd.optind
+    libc.src.unistd.optopt
+    libc.src.unistd.opterr
 
     # sys/select.h entrypoints
     libc.src.sys.select.select
   )
 endif()
 
 set(TARGET_LLVMLIBC_ENTRYPOINTS
   ${TARGET_LIBC_ENTRYPOINTS}
   ${TARGET_LIBM_ENTRYPOINTS}
 )

libc/config/linux/api.td

def DirentAPI : PublicAPI<"dirent.h"> {
   let Types = [
     "ino_t",
     "DIR",
     "struct dirent",
   ];
 }
 
 def UniStdAPI : PublicAPI<"unistd.h"> {
-  let Types = ["__exec_argv_t", "__exec_envp_t", "off_t", "pid_t", "size_t", "ssize_t", "uid_t"];
+  let Types = ["__exec_argv_t", "__exec_envp_t", "off_t", "pid_t", "size_t", "ssize_t", "uid_t", "__getoptargv_t"];
 }
 
 def SysRandomAPI : PublicAPI<"sys/random.h"> {
   let Types = ["size_t", "ssize_t"];
 }
 
 def SysSelectAPI : PublicAPI<"sys/select.h"> {
   let Types = ["fd_set", "sigset_t", "suseconds_t", "time_t", "struct timespec", "struct timeval"];

libc/config/linux/x86_64/entrypoints.txt

list(APPEND TARGET_LIBC_ENTRYPOINTS
     libc.src.time.nanosleep
     libc.src.time.time
 
     # unistd.h entrypoints
     libc.src.unistd.environ
     libc.src.unistd.execv
     libc.src.unistd.fork
     libc.src.unistd.__llvm_libc_syscall
+    libc.src.unistd.getopt
+    libc.src.unistd.optarg
+    libc.src.unistd.optind
+    libc.src.unistd.optopt
+    libc.src.unistd.opterr
 
     # sys/select.h entrypoints
     libc.src.sys.select.select
   )
 endif()
 
 set(TARGET_LLVMLIBC_ENTRYPOINTS
   ${TARGET_LIBC_ENTRYPOINTS}
   ${TARGET_LIBM_ENTRYPOINTS}
 )

libc/include/CMakeLists.txt

DEPENDS
     .llvm-libc-macros.unistd_macros
     .llvm-libc-types.__exec_argv_t
     .llvm-libc-types.__exec_envp_t
     .llvm-libc-types.off_t
     .llvm-libc-types.pid_t
     .llvm-libc-types.size_t
     .llvm-libc-types.ssize_t
     .llvm-libc-types.uid_t
+    .llvm-libc-types.__getoptargv_t
 )
 
 add_gen_header(
   pthread
   DEF_FILE pthread.h.def
   GEN_HDR pthread.h
   DEPENDS
     .llvm_libc_common_h

libc/include/llvm-libc-types/CMakeLists.txt

 add_header(thrd_start_t HDR thrd_start_t.h)
 add_header(thrd_t HDR thrd_t.h DEPENDS .__thread_type)
 add_header(tss_t HDR tss_t.h)
 add_header(tss_dtor_t HDR tss_dtor_t.h)
 add_header(__atexithandler_t HDR __atexithandler_t.h)
 add_header(speed_t HDR speed_t.h)
 add_header(tcflag_t HDR tcflag_t.h)
 add_header(struct_termios HDR struct_termios.h DEPENDS .cc_t .speed_t .tcflag_t)
+add_header(__getoptargv_t HDR __getoptargv_t.h)

libc/include/llvm-libc-types/__getoptargv_t.h

+//===-- Definition of type __getoptargv_t ---------------------------------===//
+//
+// 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_TYPES_GETOPTARGV_T_H__
+#define __LLVM_LIBC_TYPES_GETOPTARGV_T_H__
+
+typedef char *const __getoptargv_t[];
+
+#endif // __LLVM_LIBC_TYPES_GETOPTARGV_T_H__

libc/spec/posix.td

 def StackTPtr : PtrType<StackT>;
 def RestrictedStackTPtr : RestrictedPtrType<StackT>;
 def ConstRestrictedStackTPtr : ConstType<RestrictedStackTPtr>;
 
 def FdSet : NamedType<"fd_set">;
 def FdSetPtr : PtrType<FdSet>;
 def RestrictedFdSetPtr : RestrictedPtrType<FdSet>;
 
+def GetoptArgvT : NamedType<"__getoptargv_t">;
+
 def POSIX : StandardSpec<"POSIX"> {
   PtrType CharPtr = PtrType<CharType>;
   RestrictedPtrType RestrictedCharPtr = RestrictedPtrType<CharType>;
   RestrictedPtrType CharRestrictedDoublePtr = RestrictedPtrType<CharPtr>;
   ConstType ConstCharPtr = ConstType<CharPtr>;
   ConstType ConstRestrictedCharPtr = ConstType<RestrictedCharPtr>;
 
   NamedType ModeTType = NamedType<"mode_t">;
HeaderSpec UniStd = HeaderSpec<
     [
       ExecArgvT,
       ExecEnvpT,
       OffTType,
       SSizeTType,
       SizeTType,
       PidT,
       UidT,
+      GetoptArgvT,
     ],
     [], // Enumerations
     [
         FunctionSpec<
           "access",
           RetValSpec<IntType>,
           [ArgSpec<ConstCharPtr>, ArgSpec<IntType>]
         >,
[
           RetValSpec<IntType>,
           [ArgSpec<IntType>, ArgSpec<ConstCharPtr>, ArgSpec<IntType>]
         >,
         FunctionSpec<
           "write",
           RetValSpec<SSizeTType>,
           [ArgSpec<IntType>, ArgSpec<ConstVoidPtr>, ArgSpec<SizeTType>]
         >,
+        FunctionSpec<
+          "getopt",
+          RetValSpec<IntType>,
+          [ArgSpec<IntType>, ArgSpec<GetoptArgvT>, ArgSpec<ConstCharPtr>]
+        >,
     ],
     [
         ObjectSpec<"environ", "char **">,
+        ObjectSpec<
+          "optarg",
+          "char *"
+        >,
+        ObjectSpec<
+          "optind",
+          "int"
+        >,
+        ObjectSpec<
+          "opterr",
+          "int"
+        >,
+        ObjectSpec<
+          "optopt",
+          "int"
+        >,
     ]
   >;
 
   HeaderSpec StdLib = HeaderSpec<
     "stdlib.h",
     [], // Macros
     [], // Types
     [], // Enumerations

libc/src/unistd/CMakeLists.txt

 
 add_entrypoint_object(
   environ
   SRCS
     environ.cpp
   HDRS
     environ.h
 )
+
+add_entrypoint_object(
+  getopt
+  SRCS
+    getopt.cpp
+  HDRS
+    getopt.h
+  DEPENDS
+    libc.include.unistd
+    libc.src.__support.CPP.optional
+    libc.src.__support.CPP.string_view
+    libc.src.__support.File.file
+    libc.src.stdio.fprintf
+)
+
+# These aren't actual external per-se, but this is just an easy way to create
+# targets that don't do anything. They exist to be referenced in entrypoints.txt
+# so that the header will properly expose their definitions. Their declaration
+# is in getopt.cpp.
+add_entrypoint_external(
+  optarg
+)
+
+add_entrypoint_external(
+  optind
+)
+
+add_entrypoint_external(
+  optopt
+)
+
+add_entrypoint_external(
+  opterr
+)

libc/src/unistd/getopt.h

+//===-- Implementation header for getopt ------------------------*- 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_UNISTD_GETOPT_H
+#define LLVM_LIBC_SRC_UNISTD_GETOPT_H
+
+#include <stdio.h>
+#include <unistd.h>
+
+namespace __llvm_libc {
+
+namespace impl {
+void set_getopt_state(char **, int *, int *, unsigned *, int, FILE *);
+}
+
+int getopt(int argc, char *const argv[], const char *optstring);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_UNISTD_GETOPT_H

libc/src/unistd/getopt.cpp

+//===-- Implementation of getopt ------------------------------------------===//
+//
+// 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/unistd/getopt.h"
+#include "src/__support/CPP/optional.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/File/file.h"
+#include "src/__support/common.h"
+#include "src/stdio/fprintf.h"
+
+#include <stdio.h>
+
+// This is POSIX compliant and does not support GNU extensions, mainly this is
+// just the re-ordering of argv elements such that unknown arguments can be
+// easily iterated over.
+
+namespace __llvm_libc {
+
+template <typename T> struct RefWrapper {
+  RefWrapper(T *ptr) : ptr(ptr) {}
+  RefWrapper &operator=(const RefWrapper &) = default;
+  operator T &() { return *ptr; }
+  T &get() { return *ptr; }
+  T *ptr;
+};
+
+struct GetoptContext {
+  RefWrapper<char *> optarg;
+  RefWrapper<int> optind;
+  RefWrapper<int> optopt;
+  RefWrapper<unsigned> optpos;
+
+  int opterr;
+
+  FILE *errstream;
+
+  GetoptContext &operator=(const GetoptContext &) = default;
+
+  template <typename... Ts> void report_error(const char *fmt, Ts... ts) {
+    if (opterr)
+      __llvm_libc::fprintf(errstream, fmt, ts...);
+  }
+};
+
+struct OptstringParser {
+  using value_type = struct {
+    char c;
+    bool arg;
+  };
+
+  cpp::string_view optstring;
+
+  struct iterator {
+    cpp::string_view curr;
+
+    iterator operator++() {
+      curr = curr.substr(1);
+      return *this;
+    }
+
+    bool operator!=(iterator other) { return curr.data() != other.curr.data(); }
+
+    value_type operator*() {
+      value_type r{curr.front(), false};
+      if (!curr.substr(1).empty() && curr.substr(1).front() == ':') {
+        this->operator++();
+        r.arg = true;
+      }
+      return r;
+    }
+  };
+
+  iterator begin() {
+    bool skip = optstring.front() == '-' || optstring.front() == '+' ||
+                optstring.front() == ':';
+    return {optstring.substr(!!skip)};
+  }
+
+  iterator end() { return {optstring.substr(optstring.size())}; }
+};
+
+int getopt_r(int argc, char *const argv[], const char *optstring,
+             GetoptContext &ctx) {
+  auto failure = [&ctx](int ret = -1) {
+    ctx.optpos.get() = 0;
+    return ret;
+  };
+
+  if (ctx.optind >= argc || !argv[ctx.optind])
+    return failure();
+
+  cpp::string_view current =
+      cpp::string_view{argv[ctx.optind]}.substr(ctx.optpos);
+
+  auto move_forward = [&current, &ctx] {
+    current = current.substr(1);
+    ctx.optpos.get()++;
+  };
+
+  // If optpos is nonzero, then we are already parsing a valid flag and these
+  // need not be checked.
+  if (ctx.optpos == 0) {
+    if (current[0] != '-')
+      return failure();
+
+    if (current == "--") {
+      ctx.optind.get()++;
+      return failure();
+    }
+
+    // Eat the '-' char.
+    move_forward();
+    if (current.empty())
+      return failure();
+  }
+
+  auto find_match =
+      [current, optstring]() -> cpp::optional<OptstringParser::value_type> {
+    for (auto i : OptstringParser{optstring})
+      if (i.c == current[0])
+        return i;
+    return {};
+  };
+
+  auto match = find_match();
+  if (!match) {
+    ctx.report_error("%s: illegal option -- %c\n", argv[0], current[0]);
+    ctx.optopt.get() = current[0];
+    return failure('?');
+  }
+
+  // We've matched so eat that character.
+  move_forward();
+  if (match->arg) {
+    // If we found an option that takes an argument and our current is not over,
+    // the rest of current is that argument. Ie, "-cabc" with opstring "c:",
+    // then optarg should point to "abc". Otherwise the argument to c will be in
+    // the next arg like "-c abc".
+    if (!current.empty()) {
+      // This const cast is fine because current was already holding a mutable
+      // string, it just doesn't have the semantics to note that, we could use
+      // span but it doesn't have string_view string niceties.
+      ctx.optarg.get() = const_cast<char *>(current.data());
+    } else {
+      // One char lookahead to see if we ran out of arguments. If so, return ':'
+      // if the first character of optstring is ':'. optind must stay at the
+      // current value so only increase it after we known there is another arg.
+      if (ctx.optind + 1 >= argc || !argv[ctx.optind + 1]) {
+        ctx.report_error("%s: option requires an argument -- %c\n", argv[0],
+                         match->c);
+        return failure(optstring[0] == ':' ? ':' : '?');
+      }
+      ctx.optarg.get() = argv[++ctx.optind];
+    }
+    ctx.optind++;
+    ctx.optpos.get() = 0;
+  } else if (current.empty()) {
+    // If this argument is now empty we are safe to move onto the next one.
+    ctx.optind++;
+    ctx.optpos.get() = 0;
+  }
+
+  return match->c;
+}
+
+namespace impl {
+
+extern "C" char *optarg;
+extern "C" int optind = 1;
+extern "C" int optopt;
+extern "C" int opterr;
+
+static unsigned optpos;
+
+static GetoptContext ctx{
+    &impl::optarg, &impl::optind,
+    &impl::optopt, &optpos,
+    impl::opterr,  reinterpret_cast<FILE *>(__llvm_libc::stderr)};
+
+#ifndef LLVM_LIBC_PUBLIC_PACKAGING
+// This is used exclusively in tests.
+void set_getopt_state(char **optarg, int *optind, int *optopt, unsigned *optpos,
+                      int opterr, FILE *errstream) {
+  ctx = {optarg, optind, optopt, optpos, opterr, errstream};
+}
+#endif
+
+} // namespace impl
+
+LLVM_LIBC_FUNCTION(int, getopt,
+                   (int argc, char *const argv[], const char *optstring)) {
+  return getopt_r(argc, argv, optstring, impl::ctx);
+}
+
+} // namespace __llvm_libc

libc/test/src/unistd/CMakeLists.txt

add_libc_unittest(
   SUITE
     libc_unistd_unittests
   SRCS
     sysconf_test.cpp
   DEPENDS
     libc.include.unistd
     libc.src.unistd.sysconf
 )
+
+add_libc_unittest(
+  getopt_test
+  SUITE
+    libc_unistd_unittests
+  SRCS
+    getopt_test.cpp
+  DEPENDS
+    libc.src.unistd.getopt
+    libc.src.__support.CPP.array
+    libc.src.stdio.fopencookie
+    libc.src.stdio.fflush
+)

libc/test/src/unistd/getopt_test.cpp

+//===-- Unittests for getopt ----------------------------------------------===//
+//
+// 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/unistd/getopt.h"
+#include "utils/UnitTest/Test.h"
+
+#include "src/__support/CPP/array.h"
+#include "src/stdio/fflush.h"
+#include "src/stdio/fopencookie.h"
+
+#include <stdio.h>
+
+using __llvm_libc::cpp::array;
+
+namespace test_globals {
+char *optarg;
+int optind = 1;
+int optopt;
+int opterr = 1;
+
+unsigned optpos;
+} // namespace test_globals
+
+// This can't be a constructor because it will get run before the constructor
+// which sets the default state in getopt.
+void set_state(FILE *errstream) {
+  __llvm_libc::impl::set_getopt_state(
+      &test_globals::optarg, &test_globals::optind, &test_globals::optopt,
+      &test_globals::optpos, test_globals::opterr, errstream);
+}
+
+// TODO: <stdio> could be either llvm-libc's or the system libc's. The former
+// doesn't currently support fmemopen but does have fopencookie. In the future
+// just use that instead. This memopen does no error checking for the size
+// of the buffer, etc.
+FILE *memopen(char **pos) {
+  static auto memcpy = [](char *dest, const char *src, size_t size) {
+    for (size_t i = 0; i < size; i++)
+      dest[i] = src[i];
+  };
+
+  static auto *write =
+      +[](void *cookie, const char *buf, size_t size) -> ssize_t {
+    char **pos = static_cast<char **>(cookie);
+    memcpy(*pos, buf, size);
+    *pos += size;
+    return size;
+  };
+
+  static cookie_io_functions_t cookie{nullptr, write, nullptr, nullptr};
+  return __llvm_libc::fopencookie(pos, "w", cookie);
+}
+
+struct LlvmLibcGetoptTest : public __llvm_libc::testing::Test {
+  FILE *errstream;
+  char buf[256];
+  char *pos = buf;
+
+  void reset_errstream() { pos = buf; }
+  const char *get_error_msg() {
+    __llvm_libc::fflush(errstream);
+    return buf;
+  }
+
+  void SetUp() override {
+    ASSERT_TRUE(!!(errstream = memopen(&pos)));
+    set_state(errstream);
+    ASSERT_EQ(test_globals::optind, 1);
+  }
+
+  void TearDown() override {
+    test_globals::optind = 1;
+    test_globals::opterr = 1;
+  }
+};
+
+// This is safe because getopt doesn't currently permute argv like GNU's getopt
+// does so this just helps silence warnings.
+char *operator"" _c(const char *c, size_t) { return const_cast<char *>(c); }
+
+TEST_F(LlvmLibcGetoptTest, NoMatch) {
+  array<char *, 3> argv{"prog"_c, "arg1"_c, nullptr};
+
+  // optind >= argc
+  EXPECT_EQ(__llvm_libc::getopt(1, argv.data(), "..."), -1);
+
+  // argv[optind] == nullptr
+  test_globals::optind = 2;
+  EXPECT_EQ(__llvm_libc::getopt(100, argv.data(), "..."), -1);
+
+  // argv[optind][0] != '-'
+  test_globals::optind = 1;
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "a"), -1);
+  ASSERT_EQ(test_globals::optind, 1);
+
+  // argv[optind] == "-"
+  argv[1] = "-"_c;
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "a"), -1);
+  ASSERT_EQ(test_globals::optind, 1);
+
+  // argv[optind] == "--", then return -1 and incremement optind
+  argv[1] = "--"_c;
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "a"), -1);
+  EXPECT_EQ(test_globals::optind, 2);
+}
+
+TEST_F(LlvmLibcGetoptTest, WrongMatch) {
+  array<char *, 3> argv{"prog"_c, "-b"_c, nullptr};
+
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "a"), int('?'));
+  EXPECT_EQ(test_globals::optopt, (int)'b');
+  EXPECT_EQ(test_globals::optind, 1);
+  EXPECT_STREQ(get_error_msg(), "prog: illegal option -- b\n");
+}
+
+TEST_F(LlvmLibcGetoptTest, OpterrFalse) {
+  array<char *, 3> argv{"prog"_c, "-b"_c, nullptr};
+
+  test_globals::opterr = 0;
+  set_state(errstream);
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "a"), int('?'));
+  EXPECT_EQ(test_globals::optopt, (int)'b');
+  EXPECT_EQ(test_globals::optind, 1);
+  EXPECT_STREQ(get_error_msg(), "");
+}
+
+TEST_F(LlvmLibcGetoptTest, MissingArg) {
+  array<char *, 3> argv{"prog"_c, "-b"_c, nullptr};
+
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), ":b:"), (int)':');
+  ASSERT_EQ(test_globals::optind, 1);
+  EXPECT_STREQ(get_error_msg(), "prog: option requires an argument -- b\n");
+  reset_errstream();
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "b:"), int('?'));
+  EXPECT_EQ(test_globals::optind, 1);
+  EXPECT_STREQ(get_error_msg(), "prog: option requires an argument -- b\n");
+}
+
+TEST_F(LlvmLibcGetoptTest, ParseArgInCurrent) {
+  array<char *, 3> argv{"prog"_c, "-barg"_c, nullptr};
+
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "b:"), (int)'b');
+  EXPECT_STREQ(test_globals::optarg, "arg");
+  EXPECT_EQ(test_globals::optind, 2);
+}
+
+TEST_F(LlvmLibcGetoptTest, ParseArgInNext) {
+  array<char *, 4> argv{"prog"_c, "-b"_c, "arg"_c, nullptr};
+
+  EXPECT_EQ(__llvm_libc::getopt(3, argv.data(), "b:"), (int)'b');
+  EXPECT_STREQ(test_globals::optarg, "arg");
+  EXPECT_EQ(test_globals::optind, 3);
+}
+
+TEST_F(LlvmLibcGetoptTest, ParseMutliInOne) {
+  array<char *, 3> argv{"prog"_c, "-abc"_c, nullptr};
+
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "abc"), (int)'a');
+  ASSERT_EQ(test_globals::optind, 1);
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "abc"), (int)'b');
+  ASSERT_EQ(test_globals::optind, 1);
+  EXPECT_EQ(__llvm_libc::getopt(2, argv.data(), "abc"), (int)'c');
+  EXPECT_EQ(test_globals::optind, 2);
+}