Skip to content

[libc][NFC] unify startup library's code style with the rest #74041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

SchrodingerZhu
Copy link
Contributor

This PR unifies the startup library's code style with the rest of libc.

@llvmbot llvmbot added the libc label Dec 1, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

This PR unifies the startup library's code style with the rest of libc.


Full diff: https://github.com/llvm/llvm-project/pull/74041.diff

5 Files Affected:

  • (modified) libc/config/linux/app.h (+2-2)
  • (modified) libc/src/stdlib/getenv.cpp (+1-1)
  • (modified) libc/startup/linux/aarch64/start.cpp (+9-9)
  • (modified) libc/startup/linux/riscv/start.cpp (+9-9)
  • (modified) libc/startup/linux/x86_64/start.cpp (+27-26)
diff --git a/libc/config/linux/app.h b/libc/config/linux/app.h
index b17026a7832a3c4..0d2f9475c10db64 100644
--- a/libc/config/linux/app.h
+++ b/libc/config/linux/app.h
@@ -69,7 +69,7 @@ struct Args {
 // Data structure which captures properties of a linux application.
 struct AppProperties {
   // Page size used for the application.
-  uintptr_t pageSize;
+  uintptr_t page_size;
 
   Args *args;
 
@@ -77,7 +77,7 @@ struct AppProperties {
   TLSImage tls;
 
   // Environment data.
-  EnvironType *envPtr;
+  EnvironType *env_ptr;
 };
 
 extern AppProperties app;
diff --git a/libc/src/stdlib/getenv.cpp b/libc/src/stdlib/getenv.cpp
index 08397e0d8057161..7a8eb1943c0a9c4 100644
--- a/libc/src/stdlib/getenv.cpp
+++ b/libc/src/stdlib/getenv.cpp
@@ -16,7 +16,7 @@
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(char *, getenv, (const char *name)) {
-  char **env_ptr = reinterpret_cast<char **>(LIBC_NAMESPACE::app.envPtr);
+  char **env_ptr = reinterpret_cast<char **>(LIBC_NAMESPACE::app.env_ptr);
 
   if (name == nullptr || env_ptr == nullptr)
     return nullptr;
diff --git a/libc/startup/linux/aarch64/start.cpp b/libc/startup/linux/aarch64/start.cpp
index 002af5313cc82dd..f233af0fc01e427 100644
--- a/libc/startup/linux/aarch64/start.cpp
+++ b/libc/startup/linux/aarch64/start.cpp
@@ -74,7 +74,7 @@ void init_tls(TLSDescriptor &tls_descriptor) {
       MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   // We cannot check the return value with MAP_FAILED as that is the return
   // of the mmap function and not the mmap syscall.
-  if (mmap_ret_val < 0 && static_cast<uintptr_t>(mmap_ret_val) > -app.pageSize)
+  if (mmap_ret_val < 0 && static_cast<uintptr_t>(mmap_ret_val) > -app.page_size)
     LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, 1);
   uintptr_t thread_ptr = uintptr_t(reinterpret_cast<uintptr_t *>(mmap_ret_val));
   uintptr_t tls_addr = thread_ptr + size_of_pointers + padding;
@@ -144,7 +144,7 @@ __attribute__((noinline)) static void do_start() {
   // value. We step over it (the "+ 1" below) to get to the env values.
   uint64_t *env_ptr = app.args->argv + app.args->argc + 1;
   uint64_t *env_end_marker = env_ptr;
-  app.envPtr = env_ptr;
+  app.env_ptr = env_ptr;
   while (*env_end_marker)
     ++env_end_marker;
 
@@ -153,19 +153,19 @@ __attribute__((noinline)) static void do_start() {
 
   // After the env array, is the aux-vector. The end of the aux-vector is
   // denoted by an AT_NULL entry.
-  Elf64_Phdr *programHdrTable = nullptr;
-  uintptr_t programHdrCount;
+  Elf64_Phdr * = nullptr;
+  uintptr_t program_hdr_count;
   for (AuxEntry *aux_entry = reinterpret_cast<AuxEntry *>(env_end_marker + 1);
        aux_entry->type != AT_NULL; ++aux_entry) {
     switch (aux_entry->type) {
     case AT_PHDR:
-      programHdrTable = reinterpret_cast<Elf64_Phdr *>(aux_entry->value);
+      = reinterpret_cast<Elf64_Phdr *>(aux_entry->value);
       break;
     case AT_PHNUM:
-      programHdrCount = aux_entry->value;
+      program_hdr_count = aux_entry->value;
       break;
     case AT_PAGESZ:
-      app.pageSize = aux_entry->value;
+      app.page_size = aux_entry->value;
       break;
     default:
       break; // TODO: Read other useful entries from the aux vector.
@@ -173,8 +173,8 @@ __attribute__((noinline)) static void do_start() {
   }
 
   app.tls.size = 0;
-  for (uintptr_t i = 0; i < programHdrCount; ++i) {
-    Elf64_Phdr *phdr = programHdrTable + i;
+  for (uintptr_t i = 0; i < program_hdr_count; ++i) {
+    Elf64_Phdr *phdr = +i;
     if (phdr->p_type != PT_TLS)
       continue;
     // TODO: p_vaddr value has to be adjusted for static-pie executables.
diff --git a/libc/startup/linux/riscv/start.cpp b/libc/startup/linux/riscv/start.cpp
index ed976d294d94215..bf04be5ad14ad1d 100644
--- a/libc/startup/linux/riscv/start.cpp
+++ b/libc/startup/linux/riscv/start.cpp
@@ -61,7 +61,7 @@ void init_tls(TLSDescriptor &tls_descriptor) {
       MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   // We cannot check the return value with MAP_FAILED as that is the return
   // of the mmap function and not the mmap syscall.
-  if (mmap_ret_val < 0 && static_cast<uintptr_t>(mmap_ret_val) > -app.pageSize)
+  if (mmap_ret_val < 0 && static_cast<uintptr_t>(mmap_ret_val) > -app.page_size)
     LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, 1);
   uintptr_t thread_ptr = uintptr_t(reinterpret_cast<uintptr_t *>(mmap_ret_val));
   uintptr_t tls_addr = thread_ptr + size_of_pointers + padding;
@@ -147,7 +147,7 @@ __attribute__((noinline)) static void do_start() {
   // value. We step over it (the "+ 1" below) to get to the env values.
   LIBC_NAMESPACE::ArgVEntryType *env_ptr = app.args->argv + app.args->argc + 1;
   LIBC_NAMESPACE::ArgVEntryType *env_end_marker = env_ptr;
-  app.envPtr = env_ptr;
+  app.env_ptr = env_ptr;
   while (*env_end_marker)
     ++env_end_marker;
 
@@ -156,19 +156,19 @@ __attribute__((noinline)) static void do_start() {
 
   // After the env array, is the aux-vector. The end of the aux-vector is
   // denoted by an AT_NULL entry.
-  PgrHdrTableType *programHdrTable = nullptr;
-  uintptr_t programHdrCount;
+  PgrHdrTableType *program_hdr_table = nullptr;
+  uintptr_t program_hdr_count;
   for (AuxEntry *aux_entry = reinterpret_cast<AuxEntry *>(env_end_marker + 1);
        aux_entry->type != AT_NULL; ++aux_entry) {
     switch (aux_entry->type) {
     case AT_PHDR:
-      programHdrTable = reinterpret_cast<PgrHdrTableType *>(aux_entry->value);
+      program_hdr_table = reinterpret_cast<PgrHdrTableType *>(aux_entry->value);
       break;
     case AT_PHNUM:
-      programHdrCount = aux_entry->value;
+      program_hdr_count = aux_entry->value;
       break;
     case AT_PAGESZ:
-      app.pageSize = aux_entry->value;
+      app.page_size = aux_entry->value;
       break;
     default:
       break; // TODO: Read other useful entries from the aux vector.
@@ -176,8 +176,8 @@ __attribute__((noinline)) static void do_start() {
   }
 
   app.tls.size = 0;
-  for (uintptr_t i = 0; i < programHdrCount; ++i) {
-    PgrHdrTableType *phdr = programHdrTable + i;
+  for (uintptr_t i = 0; i < program_hdr_count; ++i) {
+    PgrHdrTableType *phdr = program_hdr_table + i;
     if (phdr->p_type != PT_TLS)
       continue;
     // TODO: p_vaddr value has to be adjusted for static-pie executables.
diff --git a/libc/startup/linux/x86_64/start.cpp b/libc/startup/linux/x86_64/start.cpp
index af95d2702ded975..bc1b4f0487f316a 100644
--- a/libc/startup/linux/x86_64/start.cpp
+++ b/libc/startup/linux/x86_64/start.cpp
@@ -33,9 +33,9 @@ extern "C" void __stack_chk_fail() {
 namespace LIBC_NAMESPACE {
 
 #ifdef SYS_mmap2
-static constexpr long mmapSyscallNumber = SYS_mmap2;
+static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap2;
 #elif SYS_mmap
-static constexpr long mmapSyscallNumber = SYS_mmap;
+static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap;
 #else
 #error "mmap and mmap2 syscalls not available."
 #endif
@@ -54,49 +54,50 @@ void init_tls(TLSDescriptor &tls_descriptor) {
   }
 
   // We will assume the alignment is always a power of two.
-  uintptr_t tlsSize = app.tls.size & -app.tls.align;
-  if (tlsSize != app.tls.size)
-    tlsSize += app.tls.align;
+  uintptr_t tls_size = app.tls.size & -app.tls.align;
+  if (tls_size != app.tls.size)
+    tls_size += app.tls.align;
 
   // Per the x86_64 TLS ABI, the entry pointed to by the thread pointer is the
   // address of the TLS block. So, we add more size to accomodate this address
   // entry.
   // We also need to include space for the stack canary. The canary is at
   // offset 0x28 (40) and is of size uintptr_t.
-  uintptr_t tlsSizeWithAddr = tlsSize + sizeof(uintptr_t) + 40;
+  uintptr_t tls_size_with_addr = tls_size + sizeof(uintptr_t) + 40;
 
   // We cannot call the mmap function here as the functions set errno on
   // failure. Since errno is implemented via a thread local variable, we cannot
   // use errno before TLS is setup.
-  long mmapRetVal = LIBC_NAMESPACE::syscall_impl<long>(
-      mmapSyscallNumber, nullptr, tlsSizeWithAddr, PROT_READ | PROT_WRITE,
+  long mmap_retval = LIBC_NAMESPACE::syscall_impl<long>(
+      MMAP_SYSCALL_NUMBER, nullptr, tls_size_with_addr, PROT_READ | PROT_WRITE,
       MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   // We cannot check the return value with MAP_FAILED as that is the return
   // of the mmap function and not the mmap syscall.
-  if (mmapRetVal < 0 && static_cast<uintptr_t>(mmapRetVal) > -app.pageSize)
+  if (mmap_retval < 0 && static_cast<uintptr_t>(mmap_retval) > -app.page_size)
     LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, 1);
-  uintptr_t *tlsAddr = reinterpret_cast<uintptr_t *>(mmapRetVal);
+  uintptr_t *tls_addr = reinterpret_cast<uintptr_t *>(mmap_retval);
 
   // x86_64 TLS faces down from the thread pointer with the first entry
   // pointing to the address of the first real TLS byte.
-  uintptr_t endPtr = reinterpret_cast<uintptr_t>(tlsAddr) + tlsSize;
-  *reinterpret_cast<uintptr_t *>(endPtr) = endPtr;
+  uintptr_t end_ptr = reinterpret_cast<uintptr_t>(tls_addr) + tls_size;
+  *reinterpret_cast<uintptr_t *>(end_ptr) = end_ptr;
 
-  LIBC_NAMESPACE::inline_memcpy(reinterpret_cast<char *>(tlsAddr),
+  LIBC_NAMESPACE::inline_memcpy(reinterpret_cast<char *>(tls_addr),
                                 reinterpret_cast<const char *>(app.tls.address),
                                 app.tls.init_size);
-  uintptr_t *stackGuardAddr = reinterpret_cast<uintptr_t *>(endPtr + 40);
+  uintptr_t *stack_guard_addr = reinterpret_cast<uintptr_t *>(end_ptr + 40);
   // Setting the stack guard to a random value.
   // We cannot call the get_random function here as the function sets errno on
   // failure. Since errno is implemented via a thread local variable, we cannot
   // use errno before TLS is setup.
-  ssize_t stackGuardRetVal = LIBC_NAMESPACE::syscall_impl<ssize_t>(
-      SYS_getrandom, reinterpret_cast<long>(stackGuardAddr), sizeof(uint64_t),
+  ssize_t stack_guard_retval = LIBC_NAMESPACE::syscall_impl<ssize_t>(
+      SYS_getrandom, reinterpret_cast<long>(stack_guard_addr), sizeof(uint64_t),
       0);
-  if (stackGuardRetVal < 0)
+  if (stack_guard_retval < 0)
     LIBC_NAMESPACE::syscall_impl(SYS_exit, 1);
 
-  tls_descriptor = {tlsSizeWithAddr, uintptr_t(tlsAddr), endPtr};
+  tls_descriptor = {tls_size_with_addr, reinterpret_cast<uintptr_t>(tls_addr),
+                    end_ptr};
   return;
 }
 
@@ -181,7 +182,7 @@ extern "C" void _start() {
   // value. We step over it (the "+ 1" below) to get to the env values.
   uint64_t *env_ptr = app.args->argv + app.args->argc + 1;
   uint64_t *env_end_marker = env_ptr;
-  app.envPtr = env_ptr;
+  app.env_ptr = env_ptr;
   while (*env_end_marker)
     ++env_end_marker;
 
@@ -190,19 +191,19 @@ extern "C" void _start() {
 
   // After the env array, is the aux-vector. The end of the aux-vector is
   // denoted by an AT_NULL entry.
-  Elf64_Phdr *programHdrTable = nullptr;
-  uintptr_t programHdrCount;
+  Elf64_Phdr *program_hdr_table = nullptr;
+  uintptr_t program_hdr_count = 0;
   for (AuxEntry *aux_entry = reinterpret_cast<AuxEntry *>(env_end_marker + 1);
        aux_entry->type != AT_NULL; ++aux_entry) {
     switch (aux_entry->type) {
     case AT_PHDR:
-      programHdrTable = reinterpret_cast<Elf64_Phdr *>(aux_entry->value);
+      program_hdr_table = reinterpret_cast<Elf64_Phdr *>(aux_entry->value);
       break;
     case AT_PHNUM:
-      programHdrCount = aux_entry->value;
+      program_hdr_count = aux_entry->value;
       break;
     case AT_PAGESZ:
-      app.pageSize = aux_entry->value;
+      app.page_size = aux_entry->value;
       break;
     default:
       break; // TODO: Read other useful entries from the aux vector.
@@ -210,8 +211,8 @@ extern "C" void _start() {
   }
 
   app.tls.size = 0;
-  for (uintptr_t i = 0; i < programHdrCount; ++i) {
-    Elf64_Phdr *phdr = programHdrTable + i;
+  for (uintptr_t i = 0; i < program_hdr_count; ++i) {
+    Elf64_Phdr *phdr = program_hdr_table + i;
     if (phdr->p_type != PT_TLS)
       continue;
     // TODO: p_vaddr value has to be adjusted for static-pie executables.

@SchrodingerZhu
Copy link
Contributor Author

@michaelrj-google @nickdesaulniers PTAL

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SchrodingerZhu
Copy link
Contributor Author

latest commits are just rebase. merge this?

@michaelrj-google michaelrj-google merged commit a0eda10 into llvm:main Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants