[libc-commits] [libc] [libc] Fix start up crash on 32 bit systems (PR #66210)

[Mikhail R. Gadelha via libc-commits]

https://github.com/mikhailramalho created https://github.com/llvm/llvm-project/pull/66210:

This patch changes the default types of argc/argv so it's no longer a uint64_t in all systems, instead, it's now a uintptr_t, which fixes crashes in 32-bit systems that expect 32-bit types. This patch also adds two uintptr_t types (EnvironType and AuxEntryType) for the same reason.

The patch also adds a PgrHdrTableType type behind an ifdef that's Elf64_Phdr in 64-bit systems and Elf32_Phdr in 32-bit systems.

>From 1d7e4a9e0ac1447c6d581d80e1d010ba5786ec5c Mon Sep 17 00:00:00 2001
From: "Mikhail R. Gadelha" <mikhail at igalia.com>
Date: Wed, 13 Sep 2023 10:53:59 -0300
Subject: [PATCH] [libc] Fix start up crash on 32 bit systems

This patch changes the default types of argc/argv so it's no longer a
uint64_t in all systems, instead it's now a uintptr_t, which fixes
crashes in 32-bit systems that expect 32-bit types. This patch also adds
two uintptr_t types (EnvironType and AuxEntryType) for the same reason.

The patch also a PgrHdrTableType type behind an ifdef that's Elf64_Phdr
in 64-bit systems and Elf32_Phdr in 32-bit systems.
---
 libc/config/linux/app.h              |  9 ++++++---
 libc/startup/linux/riscv64/start.cpp | 24 +++++++++++++++++-------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/libc/config/linux/app.h b/libc/config/linux/app.h
index ffd0e01f7b5a457..40a45078f6c8f35 100644
--- a/libc/config/linux/app.h
+++ b/libc/config/linux/app.h
@@ -42,11 +42,14 @@ struct TLSImage {
 // ABI specifies it as an 8 byte value. Likewise, in the ARM64 ABI, arguments
 // are usually passed in registers.  x0 is a doubleword register, so this is
 // 64 bit for aarch64 as well.
-typedef uint64_t ArgcType;
+typedef uintptr_t ArgcType;
 
 // At the language level, argv is a char** value. However, we use uint64_t as
 // ABIs specify the argv vector be an |argc| long array of 8-byte values.
-typedef uint64_t ArgVEntryType;
+typedef uintptr_t ArgVEntryType;
+
+typedef uintptr_t EnvironType;
+typedef uintptr_t AuxEntryType;
 #else
 #error "argc and argv types are not defined for the target platform."
 #endif
@@ -74,7 +77,7 @@ struct AppProperties {
   TLSImage tls;
 
   // Environment data.
-  uint64_t *envPtr;
+  EnvironType *envPtr;
 };
 
 extern AppProperties app;
diff --git a/libc/startup/linux/riscv64/start.cpp b/libc/startup/linux/riscv64/start.cpp
index d68cc9219ecce7e..1bf05b94ad5acfe 100644
--- a/libc/startup/linux/riscv64/start.cpp
+++ b/libc/startup/linux/riscv64/start.cpp
@@ -118,10 +118,20 @@ 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;
+  __llvm_libc::AuxEntryType type;
+  __llvm_libc::AuxEntryType value;
 };
 
+#if defined(LIBC_TARGET_ARCH_IS_X86_64) ||                                     \
+    defined(LIBC_TARGET_ARCH_IS_AARCH64) ||                                    \
+    defined(LIBC_TARGET_ARCH_IS_RISCV64)
+typedef Elf64_Phdr PgrHdrTableType;
+#elif defined(LIBC_TARGET_ARCH_IS_RISCV32)
+typedef Elf32_Phdr PgrHdrTableType;
+#else
+#error "Program header table type is not defined for the target platform."
+#endif
+
 __attribute__((noinline)) static void do_start() {
   LIBC_INLINE_ASM(".option push\n\t"
                   ".option norelax\n\t"
@@ -135,8 +145,8 @@ __attribute__((noinline)) static void do_start() {
   // After the argv array, is a 8-byte long NULL value before the array of env
   // values. The end of the env values is marked by another 8-byte long NULL
   // 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;
+  __llvm_libc::ArgVEntryType *env_ptr = app.args->argv + app.args->argc + 1;
+  __llvm_libc::ArgVEntryType *env_end_marker = env_ptr;
   app.envPtr = env_ptr;
   while (*env_end_marker)
     ++env_end_marker;
@@ -146,13 +156,13 @@ __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;
+  PgrHdrTableType *programHdrTable = nullptr;
   uintptr_t programHdrCount;
   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);
+      programHdrTable = reinterpret_cast<PgrHdrTableType *>(aux_entry->value);
       break;
     case AT_PHNUM:
       programHdrCount = aux_entry->value;
@@ -167,7 +177,7 @@ __attribute__((noinline)) static void do_start() {
 
   app.tls.size = 0;
   for (uintptr_t i = 0; i < programHdrCount; ++i) {
-    Elf64_Phdr *phdr = programHdrTable + i;
+    PgrHdrTableType *phdr = programHdrTable + i;
     if (phdr->p_type != PT_TLS)
       continue;
     // TODO: p_vaddr value has to be adjusted for static-pie executables.