[PATCH 0/2] Enhance getopt functions to work with PIEs

[Waldemar Kozaczuk]

Waldemar Kozaczuk (2):
Move getopt* files to libc folder and convert to C++
Enhance getopt family of functions to work with PIEs

Makefile | 4 +-
core/elf.cc | 14 +++
include/osv/elf.hh | 4 +
libc/misc/getopt.cc | 125 +++++++++++++++++++++
libc/misc/getopt_long.cc | 88 +++++++++++++++
modules/tests/Makefile | 9 +-
tests/tst-getopt.cc | 232 +++++++++++++++++++++++++++++++++++++++
7 files changed, 473 insertions(+), 3 deletions(-)
create mode 100644 libc/misc/getopt.cc
create mode 100644 libc/misc/getopt_long.cc
create mode 100644 tests/tst-getopt.cc

--
2.20.1

[Waldemar Kozaczuk]

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
Makefile | 4 +--
libc/misc/getopt.cc | 77 ++++++++++++++++++++++++++++++++++++++++
libc/misc/getopt_long.cc | 61 +++++++++++++++++++++++++++++++
3 files changed, 140 insertions(+), 2 deletions(-)

create mode 100644 libc/misc/getopt.cc
create mode 100644 libc/misc/getopt_long.cc

diff --git a/Makefile b/Makefile
index 2f68d964..2ef75600 100644
--- a/Makefile
+++ b/Makefile
@@ -1329,8 +1329,8 @@ libc += misc/error.o
libc += misc/ffs.o
musl += misc/get_current_dir_name.o
libc += misc/gethostid.o
-musl += misc/getopt.o
-musl += misc/getopt_long.o
+libc += misc/getopt.o
+libc += misc/getopt_long.o
musl += misc/getsubopt.o
libc += misc/realpath.o
libc += misc/backtrace.o
diff --git a/libc/misc/getopt.cc b/libc/misc/getopt.cc
new file mode 100644
index 00000000..70a11414
--- /dev/null
+++ b/libc/misc/getopt.cc
@@ -0,0 +1,77 @@
+#include <unistd.h>
+#include <wchar.h>
+#include <string.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <libc/libc.hh>
+
+extern "C" {
+
+char *optarg;
+int optind=1, opterr=1, optopt, __optpos, __optreset=0;
+
+#define optpos __optpos
+weak_alias(__optreset, optreset);
+
+int getopt(int argc, char * const argv[], const char *optstring)
+{
+ int i;
+ wchar_t c, d;
+ int k, l;
+ char *optchar;
+
+ if (!optind || __optreset) {
+ __optreset = 0;
+ __optpos = 0;
+ optind = 1;
+ }
+
+ if (optind >= argc || !argv[optind] || argv[optind][0] != '-' || !argv[optind][1])
+ return -1;
+ if (argv[optind][1] == '-' && !argv[optind][2])
+ return optind++, -1;
+
+ if (!optpos) optpos++;
+ if ((k = mbtowc(&c, argv[optind]+optpos, MB_LEN_MAX)) < 0) {
+ k = 1;
+ c = 0xfffd; /* replacement char */
+ }
+ optchar = argv[optind]+optpos;
+ optopt = c;
+ optpos += k;
+
+ if (!argv[optind][optpos]) {
+ optind++;
+ optpos = 0;
+ }
+
+ for (i=0; (l = mbtowc(&d, optstring+i, MB_LEN_MAX)) && d!=c; i+=l>0?l:1);
+
+ if (d != c) {
+ if (optstring[0] != ':' && opterr) {
+ write(2, argv[0], strlen(argv[0]));
+ write(2, ": illegal option: ", 18);
+ write(2, optchar, k);
+ write(2, "\n", 1);
+ }
+ return '?';
+ }
+ if (optstring[i+1] == ':') {
+ if (optind >= argc) {
+ if (optstring[0] == ':') return ':';
+ if (opterr) {
+ write(2, argv[0], strlen(argv[0]));
+ write(2, ": option requires an argument: ", 31);
+ write(2, optchar, k);
+ write(2, "\n", 1);
+ }
+ return '?';
+ }
+ optarg = argv[optind++] + optpos;
+ optpos = 0;
+ }
+ return c;
+}
+
+weak_alias(getopt, __posix_getopt);
+}
diff --git a/libc/misc/getopt_long.cc b/libc/misc/getopt_long.cc
new file mode 100644
index 00000000..fbde89fc
--- /dev/null
+++ b/libc/misc/getopt_long.cc
@@ -0,0 +1,61 @@
+#include <stddef.h>
+#include <getopt.h>
+#include <stdio.h>
+
+extern "C" {
+
+extern int __optpos, __optreset;
+
+static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly)
+{
+ if (!optind || __optreset) {
+ __optreset = 0;
+ __optpos = 0;
+ optind = 1;
+ }
+ if (optind >= argc || !argv[optind] || argv[optind][0] != '-') return -1;
+ if ((longonly && argv[optind][1]) ||
+ (argv[optind][1] == '-' && argv[optind][2]))
+ {
+ int i;
+ for (i=0; longopts[i].name; i++) {
+ const char *name = longopts[i].name;
+ char *opt = argv[optind]+1;
+ if (*opt == '-') opt++;
+ for (; *name && *name == *opt; name++, opt++);
+ if (*name || (*opt && *opt != '=')) continue;
+ if (*opt == '=') {
+ if (!longopts[i].has_arg) continue;
+ optarg = opt+1;
+ } else {
+ if (longopts[i].has_arg == required_argument) {
+ if (!(optarg = argv[++optind]))
+ return ':';
+ } else optarg = NULL;
+ }
+ optind++;
+ if (idx) *idx = i;
+ if (longopts[i].flag) {
+ *longopts[i].flag = longopts[i].val;
+ return 0;
+ }
+ return longopts[i].val;
+ }
+ if (argv[optind][1] == '-') {
+ optind++;
+ return '?';
+ }
+ }
+ return getopt(argc, argv, optstring);
+}
+
+int getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
+{
+ return __getopt_long(argc, argv, optstring, longopts, idx, 0);
+}
+
+int getopt_long_only(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
+{
+ return __getopt_long(argc, argv, optstring, longopts, idx, 1);
+}
+}
--
2.20.1

[Waldemar Kozaczuk]

This patch enhances implementation of getopt() and
getopt_long() functions to work with PIEs.

Newer GCC compiler optimizes PIEs by emitting machine code
with so called copy relocations and affects how
caller (PIEs) and callee (OSv kernel) see same logical
global variables like optarg.

New implementation of getopt() and getopt_long()
detects if caller uses its (other) copies of global
variables like optind and copies its value
to the kernel copy. Likewise on return from
the getopt() function it copies values of all "output" global
variables from kernel to the equivalent caller copies of
those variables.

Fixes #689

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---

core/elf.cc | 14 +++
include/osv/elf.hh | 4 +

libc/misc/getopt.cc | 62 +++++++++--
libc/misc/getopt_long.cc | 37 ++++++-

modules/tests/Makefile | 9 +-
tests/tst-getopt.cc | 232 +++++++++++++++++++++++++++++++++++++++

6 files changed, 345 insertions(+), 13 deletions(-)
create mode 100644 tests/tst-getopt.cc

diff --git a/core/elf.cc b/core/elf.cc
index ca9226f6..84e6fca3 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -147,6 +147,20 @@ void* object::lookup(const char* symbol)
return sm.relocated_addr();
}

+void* object::cached_lookup(const char* name)
+{
+ auto name_str = std::string(name);
+ auto cached_address_it = _cached_symbols.find(name_str);
+ if (cached_address_it != _cached_symbols.end()) {
+ return cached_address_it->second;
+ }
+ else {
+ void *symbol_address = lookup(name);
+ _cached_symbols[name_str] = symbol_address;
+ return symbol_address;
+ }
+}
+
std::vector<Elf64_Shdr> object::sections()
{
size_t bytes = size_t(_ehdr.e_shentsize) * _ehdr.e_shnum;
diff --git a/include/osv/elf.hh b/include/osv/elf.hh
index 0f1792c7..86ac1cf6 100644
--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -14,6 +14,7 @@
#include <stack>
#include <memory>
#include <unordered_set>
+#include <unordered_map>
#include <osv/types.h>
#include <atomic>

@@ -349,6 +350,7 @@ public:
void run_fini_funcs();
template <typename T = void>
T* lookup(const char* name);
+ void* cached_lookup(const char* name);
dladdr_info lookup_addr(const void* addr);
bool contains_addr(const void* addr);
ulong module_index() const;
@@ -415,6 +417,8 @@ protected:
bool _is_executable;
bool is_core();

+ std::unordered_map<std::string,void*> _cached_symbols;
+
// Keep list of references to other modules, to prevent them from being
// unloaded. When this object is unloaded, the reference count of all
// objects listed here goes down, and they too may be unloaded.
diff --git a/libc/misc/getopt.cc b/libc/misc/getopt.cc
index 70a11414..a9a749ec 100644
--- a/libc/misc/getopt.cc
+++ b/libc/misc/getopt.cc
@@ -4,6 +4,8 @@
#include <limits.h>
#include <stdlib.h>
#include <libc/libc.hh>
+#include <osv/sched.hh>
+#include <osv/app.hh>

extern "C" {

@@ -13,6 +15,30 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0;
#define optpos __optpos
weak_alias(__optreset, optreset);

+#define COPY_GLOBALS_AND_RETURN(retval) { \
+ if (other_optarg) { \
+ *other_optarg = optarg; \
+ } \
+ if (other_optind) { \
+ *other_optind = optind; \
+ }\
+ if (other_optopt) { \
+ *other_optopt = optopt; \
+ }\
+ return retval; \
+}
+
+// As explained in http://www.shrubbery.net/solaris9ab/SUNWdev/LLM/p22.html#CHAPTER4-84604
+// newer versions of gcc produce position independent executables with copies of
+// some global variables like those used by getopt() and getopt_long() for optimizations reason.
+// In those circumstances the caller of these functions uses different copies of
+// global variables (like optind) than the getopt() code that is part of OSv kernel.
+// For that reason in the beginning of the function we need to copy values of the caller
+// copies of those variables to the kernel placeholders. Likewise on every from the function
+// we need to copy the values of kernel copies of global variables to the caller ones.
+//
+// See http://man7.org/linux/man-pages/man3/getopt.3.html
+//

int getopt(int argc, char * const argv[], const char *optstring)

{
int i;
@@ -20,6 +46,26 @@ int getopt(int argc, char * const argv[], const char *optstring)
int k, l;
char *optchar;

+ char **other_optarg = nullptr;
+ int* other_optind = nullptr, *other_optopt = nullptr, *other_opterr = nullptr;
+
+ auto __runtime = sched::thread::current()->app_runtime();
+ if (__runtime) {
+ auto obj = __runtime->app.lib();
+ other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg"));
+ other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind"));
+ other_optopt = reinterpret_cast<int*>(obj->cached_lookup("optopt"));
+ other_opterr = reinterpret_cast<int*>(obj->cached_lookup("opterr"));
+ }
+
+ // Copy values of caller copies
+ if (other_opterr) {
+ opterr = *other_opterr;
+ }
+ if (other_optind) {
+ optind = *other_optind;

+ }
+
if (!optind || __optreset) {

__optreset = 0;
__optpos = 0;
@@ -27,9 +73,11 @@ int getopt(int argc, char * const argv[], const char *optstring)

}

if (optind >= argc || !argv[optind] || argv[optind][0] != '-' || !argv[optind][1])

- return -1;
- if (argv[optind][1] == '-' && !argv[optind][2])
- return optind++, -1;
+ COPY_GLOBALS_AND_RETURN(-1)

+ if (argv[optind][1] == '-' && !argv[optind][2]) {

+ optind++;
+ COPY_GLOBALS_AND_RETURN(-1)
+ }

if (!optpos) optpos++;

if ((k = mbtowc(&c, argv[optind]+optpos, MB_LEN_MAX)) < 0) {

@@ -54,23 +102,23 @@ int getopt(int argc, char * const argv[], const char *optstring)
write(2, optchar, k);
write(2, "\n", 1);
}
- return '?';
+ COPY_GLOBALS_AND_RETURN('?')

}
if (optstring[i+1] == ':') {

if (optind >= argc) {
- if (optstring[0] == ':') return ':';
+ if (optstring[0] == ':') COPY_GLOBALS_AND_RETURN(':')
if (opterr) {

write(2, argv[0], strlen(argv[0]));

write(2, ": option requires an argument: ", 31);

write(2, optchar, k);
write(2, "\n", 1);
}
- return '?';
+ COPY_GLOBALS_AND_RETURN('?')

}
optarg = argv[optind++] + optpos;

optpos = 0;
}
- return c;
+ COPY_GLOBALS_AND_RETURN(c)
}

weak_alias(getopt, __posix_getopt);
diff --git a/libc/misc/getopt_long.cc b/libc/misc/getopt_long.cc
index fbde89fc..184d22fe 100644
--- a/libc/misc/getopt_long.cc
+++ b/libc/misc/getopt_long.cc
@@ -1,6 +1,18 @@
#include <stddef.h>
#include <getopt.h>
#include <stdio.h>
+#include <osv/sched.hh>
+#include <osv/app.hh>
+
+#define COPY_GLOBALS_AND_RETURN(retval) { \
+ if (other_optarg) { \
+ *other_optarg = optarg; \
+ } \
+ if (other_optind) { \
+ *other_optind = optind; \
+ } \
+ return retval; \
+}

extern "C" {

@@ -8,12 +20,27 @@ extern int __optpos, __optreset;

static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly)
{

+ char **other_optarg = nullptr;
+ int* other_optind = nullptr;
+
+ auto __runtime = sched::thread::current()->app_runtime();
+ if (__runtime) {
+ auto obj = __runtime->app.lib();
+ other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg"));
+ other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind"));
+ }
+
+ // Copy value of caller copy
+ if (other_optind) {
+ optind = *other_optind;

+ }
+
if (!optind || __optreset) {

__optreset = 0;
__optpos = 0;
optind = 1;
}
- if (optind >= argc || !argv[optind] || argv[optind][0] != '-') return -1;
+ if (optind >= argc || !argv[optind] || argv[optind][0] != '-') COPY_GLOBALS_AND_RETURN(-1)

if ((longonly && argv[optind][1]) ||

(argv[optind][1] == '-' && argv[optind][2]))

{
@@ -30,20 +57,20 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
} else {

if (longopts[i].has_arg == required_argument) {

if (!(optarg = argv[++optind]))

- return ':';
+ COPY_GLOBALS_AND_RETURN(':')
} else optarg = NULL;
}
optind++;

if (idx) *idx = i;

if (longopts[i].flag) {

*longopts[i].flag = longopts[i].val;

- return 0;
+ COPY_GLOBALS_AND_RETURN(0)
}
- return longopts[i].val;
+ COPY_GLOBALS_AND_RETURN(longopts[i].val)

}
if (argv[optind][1] == '-') {

optind++;
- return '?';
+ COPY_GLOBALS_AND_RETURN('?')
}
}
return getopt(argc, argv, optstring);
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index 5c94e549..b0bfb031 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -62,6 +62,13 @@ $(out)/%.so: $(out)/%.o

$(out)/tests/tst-non-fpic.o: CXXFLAGS:=$(subst -fPIC,-mcmodel=large,$(CXXFLAGS))

+$(out)/tests/tst-getopt-pie.o: CXXFLAGS:=$(subst -fPIC,-fpie,$(CXXFLAGS))
+$(out)/tests/tst-getopt-pie.o: $(src)/tests/tst-getopt.cc
+ $(makedir)
+ $(call quiet, $(CXX) $(CXXFLAGS) -c -o $@ $<, CXX $*.cc)
+$(out)/tests/tst-getopt-pie.so: $(out)/tests/tst-getopt-pie.o
+ $(call quiet, $(CXX) $(CXXFLAGS) -pie -o $@ $< $(LIBS), LD $*.so)
+
# The rofs test image mounts /tmp as ramfs and 4 tests that exercise file system
# fail due to some unresolved bugs or other shortcomings of the ramfs implementation
# and are temporarily removed from the rofs-only-tests list. The tests tst-readdir.so
@@ -117,7 +124,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so tst-bsd-evh.so \
tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \
tst-sigaltstack.so tst-fread.so tst-tcp-cork.so tst-tcp-v6.so \
tst-calloc.so tst-crypt.so tst-non-fpic.so tst-small-malloc.so \
- tst-mmx-fpu.so
+ tst-mmx-fpu.so tst-getopt.so tst-getopt-pie.so
# libstatic-thread-variable.so tst-static-thread-variable.so \

tests += testrunner.so
diff --git a/tests/tst-getopt.cc b/tests/tst-getopt.cc
new file mode 100644
index 00000000..7e70f754
--- /dev/null
+++ b/tests/tst-getopt.cc
@@ -0,0 +1,232 @@
+/*
+ * Copyright (C) 2019 Waldek Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ *
+ * This source code is loosely based on the examples from GNU libs manuals
+ * https://www.gnu.org/software/libc/manual/html_node/Getopt-Long-Option-Example.html
+ * and https://www.gnu.org/software/libc/manual/html_node/Example-of-Getopt.html.
+ */
+
+#include <getopt.h>
+#include <ctype.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <cassert>
+#include <cstring>
+
+void
+test_getopt(int argc, char *const argv[], int expected_aflag, int expected_bflag,
+ const char *expected_cvalue, const char *expected_non_option_arg) {
+
+ int aflag = 0;
+ int bflag = 0;
+ char *cvalue = NULL;
+ int c;
+
+ printf("Running test: %s ...\n", argv[0]);
+
+ opterr = 0;
+ optind = 0;
+
+ while ((c = getopt(argc, argv, "abc:")) != -1)
+ switch (c) {
+ case 'a':
+ aflag = 1;
+ break;
+ case 'b':
+ bflag = 1;
+ break;
+ case 'c':
+ cvalue = optarg;
+ break;
+ case '?':
+ if (optopt == 'c')
+ fprintf(stderr, "Option -%c requires an argument.\n", optopt);
+ else if (isprint (optopt))
+ fprintf(stderr, "Unknown option `-%c'.\n", optopt);
+ else
+ fprintf(stderr,
+ "Unknown option character `\\x%x'.\n",
+ optopt);
+ assert(0);
+ default:
+ assert(0);
+ }
+
+ assert(expected_aflag == aflag);
+ assert(expected_bflag == bflag);
+ if (expected_cvalue && cvalue) {
+ assert(strcmp(expected_cvalue, cvalue) == 0);
+ } else {
+ assert(!cvalue && !expected_cvalue);
+ }
+
+ if (expected_non_option_arg) {
+ assert(optind + 1 == argc);
+ assert(strcmp(expected_non_option_arg, argv[optind]) == 0);
+ } else {
+ assert(optind == argc);
+ }
+}
+
+static int verbose_flag;
+
+void
+test_getopt_long(int argc, char *const argv[], int expected_aflag, int expected_bflag, const char *expected_cvalue,
+ const char *expected_dvalue, const char *expected_fvalue, int expected_verbose) {
+ int c;
+ int aflag = 0;
+ int bflag = 0;
+ char *cvalue = NULL;
+ char *dvalue = NULL;
+ char *fvalue = NULL;
+
+ printf("Running test: %s ...\n", argv[0]);
+
+ opterr = 0;
+ optind = 0;
+
+ while (1) {
+ struct option long_options[] =
+ {
+ /* These options set a flag. */
+ {"verbose", no_argument, &verbose_flag, 1},
+ {"brief", no_argument, &verbose_flag, 0},
+ /* These options don’t set a flag.
+ We distinguish them by their indices. */
+ {"add", no_argument, 0, 'a'},
+ {"append", no_argument, 0, 'b'},
+ {"delete", required_argument, 0, 'd'},
+ {"create", required_argument, 0, 'c'},
+ {"file", required_argument, 0, 'f'},
+ {0, 0, 0, 0}
+ };
+ /* getopt_long stores the option index here. */
+ int option_index = 0;
+
+ c = getopt_long(argc, argv, "abc:d:f:",
+ long_options, &option_index);
+
+ /* Detect the end of the options. */
+ if (c == -1)
+ break;
+
+ switch (c) {
+ case 0:
+ /* If this option set a flag, do nothing else now. */
+ if (long_options[option_index].flag != 0)
+ break;
+ printf("option %s", long_options[option_index].name);
+ if (optarg)
+ printf(" with arg %s", optarg);
+ printf("\n");
+ break;
+
+ case 'a':
+ aflag = 1;
+ break;
+
+ case 'b':
+ bflag = 1;
+ break;
+
+ case 'c':
+ printf("option -c with value `%s'\n", optarg);
+ cvalue = optarg;
+ break;
+
+ case 'd':
+ printf("option -d with value `%s'\n", optarg);
+ dvalue = optarg;
+ break;
+
+ case 'f':
+ printf("option -f with value `%s'\n", optarg);
+ fvalue = optarg;
+ break;
+
+ case '?':
+ /* getopt_long already printed an error message. */
+ assert(0);
+
+ default:
+ assert(0);
+ }
+ }
+
+ assert(expected_aflag == aflag);
+ assert(expected_bflag == bflag);
+
+ if (expected_cvalue && cvalue) {
+ assert(strcmp(expected_cvalue, cvalue) == 0);
+ } else {
+ assert(!cvalue && !expected_cvalue);
+ }
+
+ if (expected_dvalue && dvalue) {
+ assert(strcmp(expected_dvalue, dvalue) == 0);
+ } else {
+ assert(!dvalue && !expected_dvalue);
+ }
+
+ if (expected_fvalue && fvalue) {
+ assert(strcmp(expected_fvalue, fvalue) == 0);
+ } else {
+ assert(!fvalue && !expected_fvalue);
+ }
+
+ assert(verbose_flag == expected_verbose);
+
+ if (optind < argc) {
+ printf("non-option ARGV-elements: ");
+ while (optind < argc)
+ printf("%s ", argv[optind++]);
+ putchar('\n');
+ }
+}
+
+int
+main(int argc, char *argv[]) {
+ char *const test1[] = {(char *) "tst-getopt1", nullptr};
+ test_getopt(1, test1, 0, 0, nullptr, nullptr);
+
+ char *const test2[] = {(char *) "tst-getopt2", (char *) "-a", (char *) "-b", nullptr};
+ test_getopt(3, test2, 1, 1, nullptr, nullptr);
+
+ char *const test3[] = {(char *) "tst-getopt3", (char *) "-ab", nullptr};
+ test_getopt(2, test3, 1, 1, nullptr, nullptr);
+
+ char *const test4[] = {(char *) "tst-getopt4", (char *) "-c", (char *) "foo", nullptr};
+ test_getopt(3, test4, 0, 0, "foo", nullptr);
+
+ char *const test5[] = {(char *) "tst-getopt5", (char *) "-cfoo", nullptr};
+ test_getopt(2, test5, 0, 0, "foo", nullptr);
+
+ char *const test6[] = {(char *) "tst-getopt6", (char *) "arg1", nullptr};
+ test_getopt(2, test6, 0, 0, nullptr, "arg1");
+
+ char *const test7[] = {(char *) "tst-getopt7", (char *) "-a", (char *) "arg1", nullptr};
+ test_getopt(3, test7, 1, 0, nullptr, "arg1");
+
+ char *const test8[] = {(char *) "tst-getopt8", (char *) "-c", (char *) "foo", (char *) "arg1", nullptr};
+ test_getopt(4, test8, 0, 0, "foo", "arg1");
+
+ char *const test9[] = {(char *) "tst-getopt9", (char *) "-a", (char *) "--", (char *) "-b", nullptr};
+ test_getopt(4, test9, 1, 0, nullptr, "-b");
+
+ char *const test10[] = {(char *) "tst-getopt10", (char *) "-a", (char *) "-", nullptr};
+ test_getopt(3, test10, 1, 0, nullptr, "-");
+
+ char *const long_test1[] = {(char *) "tst-getopt_long1", (char *) "-a", (char *) "--create", (char *) "bula",
+ nullptr};
+ test_getopt_long(4, long_test1, 1, 0, "bula", nullptr, nullptr, 0);
+
+ char *const long_test2[] = {(char *) "tst-getopt_long2", (char *) "-a", (char *) "--create", (char *) "bula",
+ (char *) "--append", (char *) "-f", (char *) "x.txt", (char *) "--verbose", nullptr};
+ test_getopt_long(8, long_test2, 1, 1, "bula", nullptr, "x.txt", 1);
+
+ return 0;
+}
--
2.20.1

[Waldek Kozaczuk]

Ping.

[Waldek Kozaczuk]

When trying to run unmodified iperf3 3.6 (with these patches applied to OSv) and using the scripts/manifest_from_host.sh from the other patch I emailed I get this error per gdb:

(gdb) bt

#0  0x00000000003acaa2 in processor::cli_hlt () at arch/x64/processor.hh:247

#1  arch::halt_no_interrupts () at arch/x64/arch.hh:48

#2  osv::halt () at arch/x64/power.cc:26

#3  0x0000000000241614 in abort (fmt=fmt@entry=0x63511b "Aborted\n") at runtime.cc:132

#4  0x00000000002027b1 in abort () at runtime.cc:98

#5  0x0000000000349273 in mmu::vm_sigsegv (ef=0xffff800002adc068, addr=<optimized out>) at core/mmu.cc:1316

#6  mmu::vm_sigsegv (addr=<optimized out>, ef=0xffff800002adc068) at core/mmu.cc:1310

#7  0x000000000034aa9a in mmu::vm_fault (addr=addr@entry=0, ef=ef@entry=0xffff800002adc068) at core/mmu.cc:1338

#8  0x00000000003a68fe in page_fault (ef=0xffff800002adc068) at arch/x64/mmu.cc:38

#9  <signal handler called>

#10 0x000000000045a9cd in strlcpy (d=d@entry=0x2000001fdcc0 "", s=<optimized out>, s@entry=0x0, n=<optimized out>, n@entry=4096) at libc/string/strlcpy.c:21

#11 0x000000000043e39f in path_conv (wd=0xffff800001462040 "/", cpath=cpath@entry=0x0, full=full@entry=0x2000001fed00 "") at fs/vfs/vfs_task.cc:74

#12 0x000000000043e5d8 in task_conv (t=<optimized out>, cpath=cpath@entry=0x0, acc=<optimized out>, full=full@entry=0x2000001fed00 "") at fs/vfs/vfs_task.cc:149

#13 0x000000000043426f in open (pathname=pathname@entry=0x0, flags=0) at fs/vfs/main.cc:136

#14 0x00000000004534f2 in fopen (filename=0x0, mode=0x100000092604 "r") at libc/stdio/fopen.c:22

#15 0x000010000014e925 in BIO_new_file ()

#16 0x0000100000086b01 in load_privkey_from_file (file=0x0) at iperf_auth.c:182

#17 0x000010000007c518 in iperf_parse_arguments (test=0xffffa00001cf4c00, argc=2, argv=0xffffa00001ce9200) at iperf_api.c:1175

#18 0x00001000000682fe in main (argc=2, argv=0xffffa00001ce9200) at main.c:100

#19 0x000000000042f6dd in osv::application::run_main (this=0xffffa00001c3c610) at /usr/include/c++/8/bits/stl_vector.h:805

#20 0x000000000042f80f in __libc_start_main (main=0x100000068295 <main>) at core/app.cc:37

#21 0x00001000000681da in _start ()

Here is how I build and run:

./scripts/manifest_from_host.sh -w iperf3 && ./scripts/build --append-manifest
./scripts/run.py -e '/tools/iperf3 -s'

Per this fragment of iperf3 code - https://github.com/esnet/iperf/blob/88f3ad87f38762081a6d7127890ff5c3a052b06a/src/iperf_api.c#L1164-L1176 - it tries to call load_privkey_from_file () even though server_rsa_private_key is null. Is it an indicator of some subtle bug in my patch? On Linux it works just fine.

However if I add a PEM file and execute same iperf3 on OSv like that (btw the "some_users_file" does not even exist on OSv):

./scripts/run.py -e '--verbose /iperf3 -s --rsa-private-key-path /etc/private.pem --authorized-users-path some_users_file'

iperf starts:

OSv v0.53.0-14-gf07aa8e1

eth0: 192.168.122.15

Booted up in 393.29 ms

warning: this system does not seem to support IPv6 - trying IPv4

-----------------------------------------------------------

Server listening on 5201

-----------------------------------------------------------

Not sure what is going on ..

Waldek

On Tuesday, April 30, 2019 at 11:49:47 PM UTC-4, Waldek Kozaczuk wrote:

[Waldek Kozaczuk]

Did not mean to hijack this patch thread but it seemed related. In reality the issue with running unmodified iperf was caused by a bug in our fopen impl whch I sent a patch for.

[Nadav Har'El]

Nitpick: this means that on each called to cached_lookup(), even successful ones, we need to

allocate this copy of name. It's not the end of the world (getopt() is definitely not a performance

sensitive function) but if, instead, you had

void* object::cached_lookup(const std::string& name)

the extra copy would have been avoided. The caller who always calls this with a constant string,

could do something like

     static constexpr std::string optarg_str("optarg");

     ...

     cached_lookup(optarg_str);

This is a reasonable approach, but I saw you need to use this macro in a lot of places. Which is kind

of sad because first of all you need to not forget it anywhere, and second - if we ever want to upgrade

musl we'll need to fix this again in all the same places.

One way C++ can help you avoid the need to remember to do things every time you need to return

from a function is to use RAII: a local object which automatically does something when destructed.

Here is an example of something you can do instead of your macros, with a lot less changes to the

insides of the existing functions - basically just one line of change (rename getop() to static _getopt).

Please consider the following idea (but if you prefer yours, feel free to say so):

// This is a simple RAII class for retreiving the caller's copy of the global opt* variables

// on initialization, and returning them back to the caller on destruction.

class caller_optvars {

    std::shared_ptr<osv::application_runtime> _runtime;

    caller_optvars() : _runtime(sched::thread::current()->app_runtime()) {

          if (_runtime) {

              auto obj = _runtime->app.lib();

              // Note that only optind and opterr are input variables.

              optind = *reinterpret_cast<int*>(obj->cached_lookup("optind"));
              opterr = *reinterpret_cast<int*>(obj->cached_lookup("opterr")); 

         }

     }

     ~caller_optvars() {

          if (_runtime) {

              auto obj = _runtime->app.lib();

              // Note that only optind, optarg and optopt are output variables.

              // Note: Only "optind" is both input and output. We could save the result of its lookup in this

              // class. But not worth the effort.

              *reinterpret_cast<int*>(obj->cached_lookup("optind")) = optind;
              *reinterpret_cast<int*>(obj->cached_lookup("optopt")) = optopt;  

              *reinterpret_cast<char**>(obj->cached_lookup("optarg")) = optarg;

         }

     }

}

int getopt(int argc, char * const argv[], const char *optstring) {

    caller_optvars guard;

    return _getopt(argc, argv, optstring);

}

You could use the same RAII class for getopt_long() although I saw that you noticed that getopt_long()

uses slightly different variables (doesn't use opterr and optind) so it can use a different class.

+

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Waldek Kozaczuk]

Sent from my iPhone

Good idea. I was also considering guarding the cached_lookup implementation with a thread lock just in case in future we use it for other reason than getopt or maybe what if we have to apps starting and suing getopt. But this we would not support anyway as there is only one global variable copy on the kernel size. Plus I do not think it is a issue given OSv is primarily unikernel and running two or more executables is not really supported because of the limitations like this. 

Your approach is way more fancy than mine but I definitely like yours much better :-) it is always nice to learn from the examples like this. 

I think the assignments in both constructor and destructor need to be more defensive and check if symbol copy of optind or other actually exists. 

[Nadav Har'El]

Does obj->cached_lookup("optarg") look up only in the specific object, not in all its dependencies (including the kernel)?

I was hoping it's the latter, but don't remember...

[Waldemar Kozaczuk]

This patch enhances implementation of getopt() and
getopt_long() functions to work with PIEs.

Newer GCC compiler optimizes PIEs by emitting machine code
with so called copy relocations and affects how
caller (PIEs) and callee (OSv kernel) see same logical
global variables like optarg.

New implementation of getopt() and getopt_long()
detects if caller uses its (other) copies of global
variables like optind and copies its value
to the kernel copy. Likewise on return from
the getopt() function it copies values of all "output" global
variables from kernel to the equivalent caller copies of
those variables.

Fixes #689

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---

core/elf.cc | 13 +++
include/osv/elf.hh | 4 +
libc/misc/getopt.cc | 54 ++++++++-
libc/misc/getopt.hh | 21 ++++
libc/misc/getopt_long.cc | 7 +-

modules/tests/Makefile | 9 +-
tests/tst-getopt.cc | 232 +++++++++++++++++++++++++++++++++++++++

7 files changed, 337 insertions(+), 3 deletions(-)
create mode 100644 libc/misc/getopt.hh

create mode 100644 tests/tst-getopt.cc

diff --git a/core/elf.cc b/core/elf.cc

index ca9226f6..552622cf 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -147,6 +147,19 @@ void* object::lookup(const char* symbol)
return sm.relocated_addr();
}

+void* object::cached_lookup(const std::string& name)
+{
+ auto cached_address_it = _cached_symbols.find(name);

+ if (cached_address_it != _cached_symbols.end()) {
+ return cached_address_it->second;
+ }
+ else {

+ void *symbol_address = lookup(name.c_str());
+ _cached_symbols[name] = symbol_address;

+ return symbol_address;
+ }
+}
+
std::vector<Elf64_Shdr> object::sections()
{
size_t bytes = size_t(_ehdr.e_shentsize) * _ehdr.e_shnum;
diff --git a/include/osv/elf.hh b/include/osv/elf.hh

index 0f1792c7..8c7441b6 100644

--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -14,6 +14,7 @@
#include <stack>
#include <memory>
#include <unordered_set>
+#include <unordered_map>
#include <osv/types.h>
#include <atomic>

@@ -349,6 +350,7 @@ public:
void run_fini_funcs();
template <typename T = void>
T* lookup(const char* name);

+ void* cached_lookup(const std::string& name);

dladdr_info lookup_addr(const void* addr);
bool contains_addr(const void* addr);
ulong module_index() const;
@@ -415,6 +417,8 @@ protected:
bool _is_executable;
bool is_core();

+ std::unordered_map<std::string,void*> _cached_symbols;
+
// Keep list of references to other modules, to prevent them from being
// unloaded. When this object is unloaded, the reference count of all
// objects listed here goes down, and they too may be unloaded.
diff --git a/libc/misc/getopt.cc b/libc/misc/getopt.cc

index 70a11414..8fc708fa 100644
--- a/libc/misc/getopt.cc
+++ b/libc/misc/getopt.cc
@@ -4,6 +4,42 @@

#include <limits.h>
#include <stdlib.h>
#include <libc/libc.hh>
+#include <osv/sched.hh>

+#include "getopt.hh"
+
+// This is a simple RAII class for retrieving the caller's copy of the global opt* variables
+// on initialization, and returning them back to the caller on destruction.
+getopt_caller_vars_copier::getopt_caller_vars_copier() : _runtime(sched::thread::current()->app_runtime()) {
+ if (_runtime) {
+ auto obj = _runtime->app.lib();
+ other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind"));

+ if (other_optind) {
+ optind = *other_optind;
+ }
+

+ auto other_opterr = reinterpret_cast<int*>(obj->cached_lookup("opterr"));

+ if (other_opterr) {
+ opterr = *other_opterr;
+ }
+ }

+}
+
+getopt_caller_vars_copier::~getopt_caller_vars_copier() {
+ if (_runtime) {
+ auto obj = _runtime->app.lib();

+ if (other_optind) {
+ *other_optind = optind;
+ }

+ auto other_optopt = reinterpret_cast<int*>(obj->cached_lookup("optopt"));

+ if (other_optopt) {
+ *other_optopt = optopt;
+ }

+ auto other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg"));

+ if (other_optarg) {
+ *other_optarg = optarg;
+ }
+ }

+}

extern "C" {

@@ -13,7 +49,7 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0;

#define optpos __optpos
weak_alias(__optreset, optreset);

-int getopt(int argc, char * const argv[], const char *optstring)
+int __getopt(int argc, char * const argv[], const char *optstring)
{
int i;
wchar_t c, d;
@@ -73,5 +109,21 @@ int getopt(int argc, char * const argv[], const char *optstring)
return c;

}

+// As explained in http://www.shrubbery.net/solaris9ab/SUNWdev/LLM/p22.html#CHAPTER4-84604
+// newer versions of gcc produce position independent executables with copies of
+// some global variables like those used by getopt() and getopt_long() for optimizations reason.
+// In those circumstances the caller of these functions uses different copies of
+// global variables (like optind) than the getopt() code that is part of OSv kernel.
+// For that reason in the beginning of the function we need to copy values of the caller
+// copies of those variables to the kernel placeholders. Likewise on every from the function
+// we need to copy the values of kernel copies of global variables to the caller ones.
+//
+// See http://man7.org/linux/man-pages/man3/getopt.3.html

+int getopt(int argc, char * const argv[], const char *optstring)
+{
+ getopt_caller_vars_copier guard;
+ return __getopt(argc, argv, optstring);
+}
+
weak_alias(getopt, __posix_getopt);
}
diff --git a/libc/misc/getopt.hh b/libc/misc/getopt.hh
new file mode 100644
index 00000000..0bf979cc
--- /dev/null
+++ b/libc/misc/getopt.hh
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk

+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.

+ */
+
+#ifndef GETOPT_HH_
+#define GETOPT_HH_
+
+#include <osv/app.hh>
+
+class getopt_caller_vars_copier {
+ std::shared_ptr<osv::application_runtime> _runtime;
+ int *other_optind;
+
+public:
+ getopt_caller_vars_copier();
+ ~getopt_caller_vars_copier();
+};
+#endif
diff --git a/libc/misc/getopt_long.cc b/libc/misc/getopt_long.cc
index fbde89fc..ac2b9cbe 100644
--- a/libc/misc/getopt_long.cc
+++ b/libc/misc/getopt_long.cc
@@ -1,11 +1,14 @@

#include <stddef.h>
#include <getopt.h>
#include <stdio.h>

+#include "getopt.hh"

extern "C" {

extern int __optpos, __optreset;

+extern int __getopt(int argc, char * const argv[], const char *optstring);
+

static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly)
{

if (!optind || __optreset) {
@@ -46,16 +49,18 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
return '?';
}
}
- return getopt(argc, argv, optstring);
+ return __getopt(argc, argv, optstring);
}

int getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
{
+ getopt_caller_vars_copier guard;

return __getopt_long(argc, argv, optstring, longopts, idx, 0);
}

int getopt_long_only(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
{
+ getopt_caller_vars_copier guard;

return __getopt_long(argc, argv, optstring, longopts, idx, 1);
}
}

[Nadav Har'El]

Thanks, looks mostly good. I just have a couple of nitpicks below.

If you want, you can also put this code in getopt.hh (only one copy of the code will be in the executable), but this is good too.

 extern "C" {

@@ -13,7 +49,7 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0;
 #define optpos __optpos
 weak_alias(__optreset, optreset);

-int getopt(int argc, char * const argv[], const char *optstring)
+int __getopt(int argc, char * const argv[], const char *optstring)

 {
        int i;
        wchar_t c, d;
@@ -73,5 +109,21 @@ int getopt(int argc, char * const argv[], const char *optstring)
        return c;
 }

+// As explained in http://www.shrubbery.net/solaris9ab/SUNWdev/LLM/p22.html#CHAPTER4-84604
+// newer versions of gcc produce position independent executables with copies of
+// some global variables like those used by getopt() and getopt_long() for optimizations reason.
+// In those circumstances the caller of these functions uses different copies of
+// global variables (like optind) than the getopt() code that is part of OSv kernel.
+// For that reason in the beginning of the function we need to copy values of the caller
+// copies of those variables to the kernel placeholders. Likewise on every from the function

every *return* from the function

+// we need to copy the values of kernel copies of global variables to the caller ones.
+//
+// See http://man7.org/linux/man-pages/man3/getopt.3.html

Maybe you should put this comment next to the copier class, not specifically next to getopt()?

This can be in getopt.hh, which will help make sure that you have the same definition in getopt.cc.

Too bad we need to call this, otherwise __getopt() didn't have to exist at all (we could just put the guard in the original getopt() function)

+
 static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly)
 {
        if (!optind || __optreset) {
@@ -46,16 +49,18 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
                        return '?';
                }
        }
-       return getopt(argc, argv, optstring);
+       return __getopt(argc, argv, optstring);

Oops, I didn't notice we have this recursive call :-(

So we really need to call the function without the guard, as you did here...

 }

 int getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
 {
+       getopt_caller_vars_copier guard;
        return __getopt_long(argc, argv, optstring, longopts, idx, 0);
 }

 int getopt_long_only(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)
 {
+       getopt_caller_vars_copier guard;

Nitpick: if you put the guard inside the __getopt_long() function, you wouldn't need to put it twice, in these two callers.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20190513033710.26044-1-jwkozaczuk%40gmail.com.

[Waldemar Kozaczuk]

This patch enhances implementation of getopt() and
getopt_long() functions to work with PIEs.

Newer GCC compiler optimizes PIEs by emitting machine code
with so called copy relocations and affects how
caller (PIEs) and callee (OSv kernel) see same logical
global variables like optarg.

New implementation of getopt() and getopt_long()
detects if caller uses its (other) copies of global
variables like optind and copies its value
to the kernel copy. Likewise on return from
the getopt() function it copies values of all "output" global
variables from kernel to the equivalent caller copies of
those variables.

Fixes #689

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
core/elf.cc | 13 +++
include/osv/elf.hh | 4 +

libc/misc/getopt.cc | 9 +-
libc/misc/getopt.hh | 66 +++++++++++
libc/misc/getopt_long.cc | 5 +-

modules/tests/Makefile | 9 +-
tests/tst-getopt.cc | 232 +++++++++++++++++++++++++++++++++++++++

7 files changed, 335 insertions(+), 3 deletions(-)

index 70a11414..391fcd52 100644
--- a/libc/misc/getopt.cc
+++ b/libc/misc/getopt.cc
@@ -4,6 +4,7 @@

#include <limits.h>
#include <stdlib.h>
#include <libc/libc.hh>

+#include "getopt.hh"

extern "C" {

@@ -13,7 +14,7 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0;

#define optpos __optpos
weak_alias(__optreset, optreset);

-int getopt(int argc, char * const argv[], const char *optstring)
+int __getopt(int argc, char * const argv[], const char *optstring)
{
int i;
wchar_t c, d;

@@ -73,5 +74,11 @@ int getopt(int argc, char * const argv[], const char *optstring)
return c;
}

+int getopt(int argc, char * const argv[], const char *optstring)
+{
+ getopt_caller_vars_copier guard;
+ return __getopt(argc, argv, optstring);
+}
+
weak_alias(getopt, __posix_getopt);
}
diff --git a/libc/misc/getopt.hh b/libc/misc/getopt.hh
new file mode 100644

index 00000000..3c56fd83
--- /dev/null
+++ b/libc/misc/getopt.hh
@@ -0,0 +1,66 @@

+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#ifndef GETOPT_HH_
+#define GETOPT_HH_
+
+#include <osv/app.hh>
+

+// As explained in http://www.shrubbery.net/solaris9ab/SUNWdev/LLM/p22.html#CHAPTER4-84604
+// newer versions of gcc produce position independent executables with copies of
+// some global variables like those used by getopt() and getopt_long() for optimizations reason.
+// In those circumstances the caller of these functions uses different copies of
+// global variables (like optind) than the getopt() code that is part of OSv kernel.

+// For that reason in the beginning of these functions we need to copy values of the caller
+// copies of those variables to the kernel placeholders. Likewise on every return from the function

+// we need to copy the values of kernel copies of global variables to the caller ones.
+//
+// See http://man7.org/linux/man-pages/man3/getopt.3.html

+//

+// This is a simple RAII class for retrieving the caller's copy of the global opt* variables
+// on initialization, and returning them back to the caller on destruction.

+class getopt_caller_vars_copier {
+ std::shared_ptr<osv::application_runtime> _runtime;
+ int *other_optind;
+
+public:

+ getopt_caller_vars_copier() : _runtime(sched::thread::current()->app_runtime()) {

+ if (_runtime) {
+ auto obj = _runtime->app.lib();
+ other_optind = reinterpret_cast<int*>(obj->cached_lookup("optind"));
+ if (other_optind) {
+ optind = *other_optind;
+ }
+
+ auto other_opterr = reinterpret_cast<int*>(obj->cached_lookup("opterr"));
+ if (other_opterr) {
+ opterr = *other_opterr;
+ }
+ }
+ }
+

+ ~getopt_caller_vars_copier() {

+ if (_runtime) {
+ auto obj = _runtime->app.lib();
+ if (other_optind) {
+ *other_optind = optind;
+ }
+ auto other_optopt = reinterpret_cast<int*>(obj->cached_lookup("optopt"));
+ if (other_optopt) {
+ *other_optopt = optopt;
+ }
+ auto other_optarg = reinterpret_cast<char**>(obj->cached_lookup("optarg"));
+ if (other_optarg) {
+ *other_optarg = optarg;
+ }
+ }
+ }

+};
+
+extern "C" int __getopt(int argc, char * const argv[], const char *optstring);

+
+#endif
diff --git a/libc/misc/getopt_long.cc b/libc/misc/getopt_long.cc

index fbde89fc..cb124734 100644
--- a/libc/misc/getopt_long.cc
+++ b/libc/misc/getopt_long.cc
@@ -1,6 +1,7 @@

#include <stddef.h>
#include <getopt.h>
#include <stdio.h>
+#include "getopt.hh"

extern "C" {

@@ -8,6 +9,8 @@ extern int __optpos, __optreset;

static int __getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx, int longonly)
{

+ getopt_caller_vars_copier guard;

+
if (!optind || __optreset) {
__optreset = 0;
__optpos = 0;

@@ -46,7 +49,7 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con

return '?';
}
}
- return getopt(argc, argv, optstring);
+ return __getopt(argc, argv, optstring);
}

int getopt_long(int argc, char *const *argv, const char *optstring, const struct option *longopts, int *idx)

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

Move getopt* files to libc folder and convert to C++

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
Message-Id: <20190501034939.1...@gmail.com>

---
diff --git a/Makefile b/Makefile

--- a/Makefile
+++ b/Makefile
@@ -1329,8 +1329,8 @@ libc += misc/error.o
libc += misc/ffs.o
musl += misc/get_current_dir_name.o
libc += misc/gethostid.o
-musl += misc/getopt.o
-musl += misc/getopt_long.o
+libc += misc/getopt.o
+libc += misc/getopt_long.o
musl += misc/getsubopt.o
libc += misc/realpath.o
libc += misc/backtrace.o
diff --git a/libc/misc/getopt.cc b/libc/misc/getopt.cc

--- a/libc/misc/getopt.cc

--- a/libc/misc/getopt_long.cc

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

Enhance getopt family of functions to work with PIEs

This patch enhances implementation of getopt() and
getopt_long() functions to work with PIEs.

Newer GCC compiler optimizes PIEs by emitting machine code
with so called copy relocations and affects how
caller (PIEs) and callee (OSv kernel) see same logical
global variables like optarg.

New implementation of getopt() and getopt_long()
detects if caller uses its (other) copies of global
variables like optind and copies its value
to the kernel copy. Likewise on return from
the getopt() function it copies values of all "output" global
variables from kernel to the equivalent caller copies of
those variables.

Fixes #689

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

Message-Id: <20190513113651.8...@gmail.com>

---
diff --git a/core/elf.cc b/core/elf.cc

--- a/core/elf.cc
+++ b/core/elf.cc
@@ -147,6 +147,19 @@ void* object::lookup(const char* symbol)
return sm.relocated_addr();
}

+void* object::cached_lookup(const std::string& name)
+{
+ auto cached_address_it = _cached_symbols.find(name);
+ if (cached_address_it != _cached_symbols.end()) {
+ return cached_address_it->second;
+ }
+ else {
+ void *symbol_address = lookup(name.c_str());
+ _cached_symbols[name] = symbol_address;
+ return symbol_address;
+ }
+}
+
std::vector<Elf64_Shdr> object::sections()
{
size_t bytes = size_t(_ehdr.e_shentsize) * _ehdr.e_shnum;
diff --git a/include/osv/elf.hh b/include/osv/elf.hh

--- a/libc/misc/getopt.hh

--- a/libc/misc/getopt_long.cc
+++ b/libc/misc/getopt_long.cc
@@ -1,13 +1,16 @@

#include <stddef.h>
#include <getopt.h>
#include <stdio.h>
+#include "getopt.hh"

extern "C" {

extern int __optpos, __optreset;

static int __getopt_long(int argc, char *const *argv, const char
*optstring, const struct option *longopts, int *idx, int longonly)
{
+ getopt_caller_vars_copier guard;
+
if (!optind || __optreset) {
__optreset = 0;
__optpos = 0;
@@ -46,7 +49,7 @@ static int __getopt_long(int argc, char *const *argv,
const char *optstring, con
return '?';
}
}
- return getopt(argc, argv, optstring);
+ return __getopt(argc, argv, optstring);
}

int getopt_long(int argc, char *const *argv, const char *optstring, const
struct option *longopts, int *idx)
diff --git a/modules/tests/Makefile b/modules/tests/Makefile

--- a/tests/tst-getopt.cc

[Waldek Kozaczuk]

Thanks for the review.

To unsubscribe from this group and stop receiving emails from it, send an email to osv...@googlegroups.com.