[libc-commits] [PATCH] D76412: [libc] Add a simple x86_64 linux loader.

[Alex Brachet via Phabricator via libc-commits]

abrachet added a comment.

Looks pretty good I think. It fails to link when using sanitizers though, they don't go well with `-static` I think.



================
Comment at: libc/loader/linux/x86_64/start.cpp:48
+  uint64_t *env_end_marker = env_ptr;
+  while (*env_end_marker != 0)
+    ++env_end_marker;
----------------
Maybe just `while (*env_end_marker)` or `while (*env_end_marker != nullptr)`


================
Comment at: libc/test/loader/CMakeLists.txt:63
+    POST_BUILD
+    COMMAND ${ADD_LOADER_TEST_ENV} ${test_exe} ${ADD_LOADER_TEST_ARGS}
+  )
----------------
Is this always run by /bin/sh? I sometimes use a shell (fish) which needs an `env` keyword before declaring environment variables so I'm not sure it would work here. if not could we do `sh -c ${ADD_LOADER_TEST_ENV} ...`?


================
Comment at: libc/test/loader/linux/args_test.cpp:12
+
+static bool my_strcmp(const char *lhs, const char *rhs) {
+  const char *l, *r;
----------------
Could be called `streq` maybe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76412