[libc] Add init and fini array iteration to the loader.

[abrachet]

libc/test/integration/loader/linux/cxx_globals_test.cpp

17

__cxa_atexit is a alternative for the .fini_array, it doesn't use it

24

The compiler has no requirement that this constructor be placed in the .init_array. It seems the better way to test this would be to either use __attribute__(({destructor,constructor})) on a function or __attribute__((section(secname))) on a function pointer to explicitly put things in {preinit,init,fini}_array sections. The later being the only way I would know how to get something in .preinit_array

[sivachandra]

Address comments.

libc/test/integration/loader/linux/cxx_globals_test.cpp

17

I am not sure I understand this comment. My intention here was to say we cannot yet have a non-trivial destructor as it will lead to a call to __cxa_atexit which we do not yet have.

24

All good ideas so I have incorporated them. When you said that the "compiler has no requirement that this constructor be placed in the .init_array", I suppose you mean that the object initialization can be inlined, eliminating the need for an explicit constructor call (may be at -O3)? I have expanded the constructor a little bit to reduce the chance for such a possibility. Also, when we include a non-trivial destructor, an .init_array entry will be added anyway so I have left the test below as is.

[tschuett]

If libc would support Lit tests, then you could:

1. compile the test binary exactly as you like it
2. inspect the binary with llvm-objdump
3. run the binary and observe the execution as you see fit

it would give you higher confidence than forcing loader tests into unit tests.

[sivachandra]

In D130966#3695678, @tschuett wrote:

If libc would support Lit tests, then you could:

1. compile the test binary exactly as you like it
2. inspect the binary with llvm-objdump
3. run the binary and observe the execution as you see fit

it would give you higher confidence than forcing loader tests into unit tests.

In general, we are taking small steps (sometimes incomplete) to understand the various infrastructure needs. May be one day we will start using lit-tests. But, unit tests are still very nice :)

[tschuett]

In D130966#3697042, @sivachandra wrote:

In D130966#3695678, @tschuett wrote:

If libc would support Lit tests, then you could:

1. compile the test binary exactly as you like it
2. inspect the binary with llvm-objdump
3. run the binary and observe the execution as you see fit

it would give you higher confidence than forcing loader tests into unit tests.

In general, we are taking small steps (sometimes incomplete) to understand the various infrastructure needs. May be one day we will start using lit-tests. But, unit tests are still very nice :)

No worries.

[abrachet]

Got these comments in a little late, oops

libc/loader/linux/x86_64/start.cpp

93

It's not clear to me why these take argc, argv and env. .init_array functions wouldn't take these because in the general they would only be meaningful when loading an executable and not a shared object. I suppose it isn't harmful to pass arguments to a function which doesn't take them, though it seems surprising given likely no constructor functions in the wild take these.

118

These should happen in reverse order.

libc/test/integration/loader/linux/cxx_globals_test.cpp

17

I see sorry, I thought it was specific to .fini_array

libc/test/integration/loader/linux/init_fini_array_test.cpp

39 ↗

(On Diff #449395)

If the integration tests check that the process exited normally you could return 1 from main and _Exit(0) here to ensure this is called. Additionally it might make sense to specify a priority to destructor and constructor, like destructor(num) to ensure they are called in the right order.

[sivachandra]

libc/loader/linux/x86_64/start.cpp

93

Neither do I know of any practical examples. Most libcs do not pass any args to the init functions. However, this patch matches glibc behavior.

$ cat init.cpp
#include <stdio.h>

__attribute__((constructor)) void initfunc(int argc, char **argv, char **envp) {
  printf("argc: %d\n", argc);
  for (int i = 0; i < argc; ++i) {
    printf("\targ[%d] = %s\n", i, argv[i]);
  }
}

int main() {
  return 0;
}
$ clang++ init.cpp
$ ./a.out
argc: 1
	arg[0] = ./a.out
$ ./a.out abc def ghi
argc: 4
	arg[0] = ./a.out
	arg[1] = abc
	arg[2] = def
	arg[3] = ghi

118

Oops, yes! I will fix it in the next pass. I am working on the __cxa_atexit hook up anyway so will fix it in that pass.

libc/test/integration/loader/linux/init_fini_array_test.cpp

39 ↗

(On Diff #449395)

OK - I will add more tests with destructor priority which I think would be better than what we have. Priorities + the _Exit(0) idea together will make it a complete test I think.

libc/loader/linux/x86_64/start.cpp

 }
 
 // Sets the thread pointer to |val|. Returns true on success, false on failure.
 static bool set_thread_ptr(uintptr_t val) {
   return __llvm_libc::syscall(SYS_arch_prctl, ARCH_SET_FS, val) == -1 ? false
                                                                       : true;
 }
 
+using InitCallback = void(int, char **, char **);
+using FiniCallback = void(void);
+
+extern "C" {
+// These arrays are present in the .init_array and .fini_array sections.
+// The symbols are inserted by linker when it sees references to them.
+extern uintptr_t __preinit_array_start[];
+extern uintptr_t __preinit_array_end[];
+extern uintptr_t __init_array_start[];
+extern uintptr_t __init_array_end[];
+extern uintptr_t __fini_array_start[];
+extern uintptr_t __fini_array_end[];
+}
+
+static void call_init_array_callbacks(int argc, char **argv, char **env) {
+  size_t preinit_array_size = __preinit_array_end - __preinit_array_start;
+  for (size_t i = 0; i < preinit_array_size; ++i)
+    reinterpret_cast<InitCallback *>(__preinit_array_start[i])(argc, argv, env);
+  size_t init_array_size = __init_array_end - __init_array_start;
+  for (size_t i = 0; i < init_array_size; ++i)
+    reinterpret_cast<InitCallback *>(__init_array_start[i])(argc, argv, env);
+}
+
+static void call_fini_array_callbacks() {
+  size_t fini_array_size = __fini_array_end - __fini_array_start;
+  for (size_t i = 0; i < fini_array_size; ++i)
+    reinterpret_cast<FiniCallback *>(__fini_array_start[i])();
+}
+
 } // namespace __llvm_libc
 
 using __llvm_libc::app;
 
 // TODO: Would be nice to use the aux entry structure from elf.h when available.
 struct AuxEntry {
   uint64_t type;
   uint64_t value;
extern "C" void _start() {
 
   __llvm_libc::TLSDescriptor tls;
   __llvm_libc::init_tls(tls);
   if (tls.size != 0 && !__llvm_libc::set_thread_ptr(tls.tp))
     __llvm_libc::syscall(SYS_exit, 1);
 
   __llvm_libc::self.attrib = &__llvm_libc::main_thread_attrib;
 
+  __llvm_libc::call_init_array_callbacks(
+      app.args->argc, reinterpret_cast<char **>(app.args->argv),
+      reinterpret_cast<char **>(env_ptr));
+
   int retval = main(app.args->argc, reinterpret_cast<char **>(app.args->argv),
                     reinterpret_cast<char **>(env_ptr));
+
+  __llvm_libc::call_fini_array_callbacks();
+
   __llvm_libc::cleanup_tls(tls.addr, tls.size);
   __llvm_libc::syscall(SYS_exit, retval);
 }

libc/test/integration/loader/linux/CMakeLists.txt

add_integration_test(
   SRCS
     tls_test.cpp
   DEPENDS
     libc.include.errno
     libc.include.sys_mman
     libc.src.errno.errno
     libc.src.sys.mman.mmap
 )
+
+add_integration_test(
+  cxx_globals
+  SUITE libc-loader-tests
+  LOADER
+    libc.loader.linux.crt1
+  SRCS
+    cxx_globals_test.cpp
+)

libc/test/integration/loader/linux/cxx_globals_test.cpp

+//===-- Loader test to test init and fini array iteration -----------------===//
+//
+// 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 "utils/IntegrationTest/test.h"
+
+class A {
+private:
+  int val;
+
+public:
+  A(int a) : val(a) {}
+  // TODO: when we have implementation for __cxa_atexit, an explicit definition
+  // of the destructor should be provided to test that path of registering the
+  // destructor callback for a global.
+  ~A() = default;
+  int get() const { return val; }
+};
+
+A global(0x1234);
+
+TEST_MAIN() {
+  ASSERT_EQ(global.get(), 0x1234);
+  return 0;
+}