[libc-commits] [PATCH] D130966: [libc] Add init and fini array iteration to the laoder.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/test/integration/loader/linux/cxx_globals_test.cpp:17
+  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
----------------
abrachet wrote:
> `__cxa_atexit` is a alternative for the `.fini_array`, it doesn't use it
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.


================
Comment at: libc/test/integration/loader/linux/cxx_globals_test.cpp:24
+
+A global(0x1234);
+
----------------
abrachet wrote:
> 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`
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130966/new/

https://reviews.llvm.org/D130966