[libc][uefi] add crt1 #132150

RossComputerGuy · 2025-03-20T05:52:28Z

Adds crt1.o for the UEFI platform in the LLVM C library. This makes things start to become useful.

llvmbot · 2025-03-20T05:53:01Z

@llvm/pr-subscribers-libc

Author: Tristan Ross (RossComputerGuy)

Changes

Adds crt1.o for the UEFI platform in the LLVM C library. This makes things start to become useful.

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

2 Files Affected:

(added) libc/startup/uefi/CMakeLists.txt (+46)
(added) libc/startup/uefi/crt1.cpp (+65)

diff --git a/libc/startup/uefi/CMakeLists.txt b/libc/startup/uefi/CMakeLists.txt
new file mode 100644
index 0000000000000..741081144001c
--- /dev/null
+++ b/libc/startup/uefi/CMakeLists.txt
@@ -0,0 +1,46 @@
+function(add_startup_object name)
+  cmake_parse_arguments(
+    "ADD_STARTUP_OBJECT"
+    "ALIAS" # Option argument
+    "SRC"   # Single value arguments
+    "DEPENDS;COMPILE_OPTIONS" # Multi value arguments
+    ${ARGN}
+  )
+
+  get_fq_target_name(${name} fq_target_name)
+  if(ADD_STARTUP_OBJECT_ALIAS)
+    get_fq_deps_list(fq_dep_list ${ADD_STARTUP_OBJECT_DEPENDS})
+    add_library(${fq_target_name} ALIAS ${fq_dep_list})
+    return()
+  endif()
+
+  add_object_library(
+    ${name}
+    SRCS ${ADD_STARTUP_OBJECT_SRC}
+    COMPILE_OPTIONS ${ADD_STARTUP_OBJECT_COMPILE_OPTIONS}
+    ${ADD_STARTUP_OBJECT_UNPARSED_ARGUMENTS}
+    DEPENDS ${ADD_STARTUP_OBJECT_DEPENDS}
+  )
+  set_target_properties(
+    ${fq_target_name}
+    PROPERTIES
+      OUTPUT_NAME ${name}.o
+  )
+endfunction()
+
+add_startup_object(
+  crt1
+  SRCS
+  crt1.cpp
+)
+
+add_custom_target(libc-startup)
+set(startup_components crt1)
+foreach(target IN LISTS startup_components)
+  set(fq_target_name libc.startup.uefi.${target})
+  add_dependencies(libc-startup ${fq_target_name})
+  install(FILES $<TARGET_OBJECTS:${fq_target_name}>
+          DESTINATION ${LIBC_INSTALL_LIBRARY_DIR}
+          RENAME $<TARGET_PROPERTY:${fq_target_name},OUTPUT_NAME>
+          COMPONENT libc)
+endforeach()
diff --git a/libc/startup/uefi/crt1.cpp b/libc/startup/uefi/crt1.cpp
new file mode 100644
index 0000000000000..9da8b8c1e9556
--- /dev/null
+++ b/libc/startup/uefi/crt1.cpp
@@ -0,0 +1,65 @@
+//===-- Implementation of crt for UEFI ----------------------------------===//
+//
+// 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 "include/llvm-libc-macros/stdlib-macros.h"
+#include "include/llvm-libc-types/EFI_HANDLE.h"
+#include "include/llvm-libc-types/EFI_STATUS.h"
+#include "include/llvm-libc-types/EFI_SYSTEM_TABLE.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+using InitCallback = void(void);
+using FiniCallback = void(void);
+extern "C" InitCallback *__CTOR_LIST__[];
+extern "C" FiniCallback *__DTOR_LIST__[];
+
+static void call_init_array_callbacks() {
+  unsigned long nptrs = (unsigned long)__CTOR_LIST__[0];
+  unsigned long i;
+
+  if (nptrs == ~0ul) {
+    for (nptrs = 0; __CTOR_LIST__[nptrs + 1] != 0; nptrs++)
+      ;
+  }
+
+  for (i = nptrs; i >= 1; i--) {
+    __CTOR_LIST__[i]();
+  }
+}
+
+static void call_fini_array_callbacks() {
+  unsigned long nptrs = 0;
+
+  for (nptrs = 0; __DTOR_LIST__[nptrs + 1] != 0; nptrs++)
+    ;
+
+  for (unsigned long i = nptrs; i >= 1; i--) {
+    __DTOR_LIST__[i]();
+  }
+}
+} // namespace LIBC_NAMESPACE_DECL
+
+EFI_HANDLE efi_image_handle;
+EFI_SYSTEM_TABLE *efi_system_table;
+
+extern "C" int main(int argc, char **argv, char **envp);
+
+extern "C" EFI_STATUS EfiMain(EFI_HANDLE ImageHandle,
+                              EFI_SYSTEM_TABLE *SystemTable) {
+  efi_image_handle = ImageHandle;
+  efi_system_table = SystemTable;
+
+  LIBC_NAMESPACE::call_init_array_callbacks();
+
+  main(0, NULL, NULL);
+
+  LIBC_NAMESPACE::call_fini_array_callbacks();
+  // TODO: convert the return value of main to EFI_STATUS
+  return 0; // TODO: EFI_SUCCESS
+}

petrhosek · 2025-03-20T07:33:07Z

libc/startup/uefi/crt1.cpp

+extern "C" InitCallback *__CTOR_LIST__[];
+extern "C" FiniCallback *__DTOR_LIST__[];


Can you point me to documentation for these symbols? I couldn't find anything in the documentation for Microsoft's link.

It seems to come from MinGW

jhuber6

Should be fine to start. Is it possible to support `argv / argc / envp / constructors in UEFI?

RossComputerGuy · 2025-03-21T16:21:19Z

Yes but we need malloc, free, and the EFI shell protocol to be implemented. It's not something we can get right now, I'm hoping to get it in the future and it's on the roadmap.

overmighty · 2025-03-21T16:50:17Z

libc/startup/uefi/CMakeLists.txt

+  SRCS
+  crt1.cpp


Nit: formatting.

Suggested change

SRCS

crt1.cpp

SRCS

crt1.cpp

This is done

overmighty · 2025-03-21T16:51:10Z

libc/startup/uefi/crt1.cpp

+//===-- Implementation of crt for UEFI ----------------------------------===//
+//
+// 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
+//
+//===--------------------------------------------------------------------===//


Nit: formatting.

Suggested change

//===-- Implementation of crt for UEFI ----------------------------------===//

//

// 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

//

//===--------------------------------------------------------------------===//

//===-- Implementation of crt for UEFI ------------------------------------===//

//

// 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

//

//===----------------------------------------------------------------------===//

This is done

overmighty · 2025-03-21T16:51:29Z

libc/startup/uefi/crt1.cpp

+  efi_image_handle = ImageHandle;
+  efi_system_table = SystemTable;
+
+  main(0, NULL, NULL);


Nit:

Suggested change

main(0, NULL, NULL);

main(0, nullptr, nullptr);

This is done

petrhosek · 2025-03-21T16:57:11Z

libc/startup/uefi/crt1.cpp

+EFI_HANDLE efi_image_handle;
+EFI_SYSTEM_TABLE *efi_system_table;


We shouldn't be placing new symbols into the global namespace, that's problematic from C standard perspective, so these symbols need to prefixed with __llvm_libc_.

Suggested change

EFI_HANDLE efi_image_handle;

EFI_SYSTEM_TABLE *efi_system_table;

EFI_HANDLE __llvm_libc_efi_image_handle;

EFI_SYSTEM_TABLE *__llvm_libc_efi_system_table;

Do you know if there's a common convention for accessing these symbols? I looked at EDK2 and there it seems like these symbols are usually passed through explicitly as input arguments to functions that need them, but I don't know if there are other examples other than EDK2.

There isn't a standard way so this was the compromise. I could join the UEFI forum IRC and inquire about it but I wouldn't be surprised if it's a "there is no standard" solution. My thinking was this would at least be an easily recognizable way programs could access the UEFI stuff if they need it for things like graphics or networking.

Also you need extern "C" for these symbols to avoid mangling.

I got symbol not found errors with that when I tried linking against the start file.

I got it to build correctly with extern.

petrhosek · 2025-03-21T16:59:20Z

libc/startup/uefi/crt1.cpp

+
+  main(0, NULL, NULL);
+
+  // TODO: convert the return value of main to EFI_STATUS


How involved is this? Is there a reason for not doing this right away?

I'm not entirely sure since there's no errno numbers defined so we'll have to make them up. I didn't want to complicate things too much in this PR so this is just a MVP.

There's C standard errno values if nothing else.

I was going to add something to convert the error codes, where should it live in src? Under __support/OSUtil or somewhere else? I was thinking it may not be a bad idea for programs to be able to reuse the conversion.

RossComputerGuy · 2025-03-22T00:44:34Z

Ok, there's progress but:

$ /home/ross/llvm-project/libc/build/install/bin/lld-link -out:fs/a.efi -nologo -subsystem:efi_application -entry:EfiMain -tsaware:no -dll /tmp/nix-shell.U3dsaN/a-87fa78.o install/lib/x86_64-unknown-uefi-llvm/crt1.o install/lib/x86_64-unknown-uefi-llvm/libc.a
lld-link: error: undefined symbol: unsigned long __cdecl __llvm_libc_21_0_0_git::errno_to_uefi_status(int)
>>> referenced by install/lib/x86_64-unknown-uefi-llvm/crt1.o:(EfiMain)

And it seems crt1 isn't pulling the error.cpp nor is it being compiled.

petrhosek · 2025-03-22T02:46:41Z

libc/startup/uefi/CMakeLists.txt

+  SRCS
+    crt1.cpp
+  DEPENDS
+    libc.src.__support.OSUtil.uefi.uefi_util


To have an object file depend on object files (or a library), you'd need to use relocatable linking, see https://maskray.me/blog/2022-11-21-relocatable-linking. We use this approach for Linux crt0.o, see:

llvm-project/libc/startup/linux/CMakeLists.txt

Lines 114 to 119 in 3840f78

merge_relocatable_object(

crt1

.${LIBC_TARGET_ARCHITECTURE}.start

.${LIBC_TARGET_ARCHITECTURE}.tls

.do_start

)

Unfortunately, as far as I'm aware COFF linkers don't support relocatable linking so that approach isn't going to work. The most straightforward alternative is probably to move errno_to_uefi_status definition to the header.

Gotcha, yeah I think for the purposes of this platform and the capabilities currently in lld-link, making it inline and in the header would be best.

RossComputerGuy · 2025-03-22T05:42:46Z

Ok, error codes have been added in and are working. I checked that they do come through with the UEFI shell.

michaelrj-google

There are some changes related to style needed for this patch

michaelrj-google · 2025-03-26T20:20:56Z

libc/src/__support/OSUtil/uefi/error.h

@@ -0,0 +1,96 @@
+//===---------- UEFI implementation of error utils ------------*- C++ -*-===//


nit: formatting

michaelrj-google · 2025-03-26T20:26:39Z

libc/src/__support/OSUtil/uefi/error.h

+#include "errno.h"
+#include "include/llvm-libc-types/EFI_STATUS.h"
+#include "limits.h"
+#include "src/__support/macros/config.h"


formatting, and also fix these includes. I don't see errno.h or limits.h in this folder so you should probably specify the full path for those.

Tried that and got:

FAILED: libc/startup/uefi/CMakeFiles/libc.startup.uefi.crt1.dir/crt1.cpp.o /home/ross/llvm-project/libc/build/bin/clang++ --target=x86_64-unknown-uefi-llvm -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -D_DEBUG -I/home/ross/llvm-project/libc -isystem /home/ross/llvm-project/libc/build/runtimes/runtimes-x86_64-unknown-uefi-llvm-bins/libc/include -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wno-comment -Wstring-conversion -fdiagnostics-color -g -std=gnu++17 -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -fpie -ffreestanding -DLIBC_FULL_BUILD -nostdlibinc -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -MD -MT libc/startup/uefi/CMakeFiles/libc.startup.uefi.crt1.dir/crt1.cpp.o -MF libc/startup/uefi/CMakeFiles/libc.startup.uefi.crt1.dir/crt1.cpp.o.d -o libc/startup/uefi/CMakeFiles/libc.startup.uefi.crt1.dir/crt1.cpp.o -c /home/ross/llvm-project/libc/startup/uefi/crt1.cpp In file included from /home/ross/llvm-project/libc/startup/uefi/crt1.cpp:13: /home/ross/llvm-project/libc/src/__support/OSUtil/uefi/error.h:12:10: fatal error: 'include/errno.h' file not found 12 | #include "include/errno.h" | ^~~~~~~~~~~~~~~~~

michaelrj-google · 2025-03-26T20:29:18Z

libc/src/__support/OSUtil/uefi/error.h

+    {EFI_ENCODE_WARNING(EFI_WARN_RESET_REQUIRED), EINTR},
+};
+
+static constexpr size_t uefi_status_errno_map_length =


nit: constexpr variable names should be UPPER_CASE

michaelrj-google · 2025-03-26T20:31:43Z

libc/src/__support/OSUtil/uefi/error.h

+static constexpr struct errno_efi_status_entry {
+  EFI_STATUS status;
+  int errno_value;
+} uefi_status_errno_map[] = {
+    {EFI_SUCCESS, 0},
+    {EFI_ENCODE_ERROR(EFI_LOAD_ERROR), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_INVALID_PARAMETER), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_BAD_BUFFER_SIZE), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_NOT_READY), EBUSY},
+    {EFI_ENCODE_ERROR(EFI_DEVICE_ERROR), EIO},
+    {EFI_ENCODE_ERROR(EFI_WRITE_PROTECTED), EPERM},
+    {EFI_ENCODE_ERROR(EFI_OUT_OF_RESOURCES), ENOMEM},
+    {EFI_ENCODE_ERROR(EFI_VOLUME_CORRUPTED), EROFS},
+    {EFI_ENCODE_ERROR(EFI_VOLUME_FULL), ENOSPC},
+    {EFI_ENCODE_ERROR(EFI_NO_MEDIA), ENODEV},
+    {EFI_ENCODE_ERROR(EFI_MEDIA_CHANGED), ENXIO},
+    {EFI_ENCODE_ERROR(EFI_NOT_FOUND), ENOENT},
+    {EFI_ENCODE_ERROR(EFI_ACCESS_DENIED), EACCES},
+    {EFI_ENCODE_ERROR(EFI_NO_RESPONSE), EBUSY},
+    {EFI_ENCODE_ERROR(EFI_NO_MAPPING), ENODEV},
+    {EFI_ENCODE_ERROR(EFI_TIMEOUT), EBUSY},
+    {EFI_ENCODE_ERROR(EFI_NOT_STARTED), EAGAIN},
+    {EFI_ENCODE_ERROR(EFI_ALREADY_STARTED), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_ABORTED), EFAULT},
+    {EFI_ENCODE_ERROR(EFI_ICMP_ERROR), EIO},
+    {EFI_ENCODE_ERROR(EFI_TFTP_ERROR), EIO},
+    {EFI_ENCODE_ERROR(EFI_PROTOCOL_ERROR), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_INCOMPATIBLE_VERSION), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_SECURITY_VIOLATION), EPERM},
+    {EFI_ENCODE_ERROR(EFI_CRC_ERROR), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_END_OF_MEDIA), EPIPE},
+    {EFI_ENCODE_ERROR(EFI_END_OF_FILE), EPIPE},
+    {EFI_ENCODE_ERROR(EFI_INVALID_LANGUAGE), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_COMPROMISED_DATA), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_IP_ADDRESS_CONFLICT), EINVAL},
+    {EFI_ENCODE_ERROR(EFI_HTTP_ERROR), EIO},
+    {EFI_ENCODE_WARNING(EFI_WARN_UNKNOWN_GLYPH), EINVAL},
+    {EFI_ENCODE_WARNING(EFI_WARN_DELETE_FAILURE), EROFS},
+    {EFI_ENCODE_WARNING(EFI_WARN_WRITE_FAILURE), EROFS},
+    {EFI_ENCODE_WARNING(EFI_WARN_BUFFER_TOO_SMALL), E2BIG},
+    {EFI_ENCODE_WARNING(EFI_WARN_STALE_DATA), EINVAL},
+    {EFI_ENCODE_WARNING(EFI_WARN_FILE_SYSTEM), EROFS},
+    {EFI_ENCODE_WARNING(EFI_WARN_RESET_REQUIRED), EINTR},
+};


This seems similar to what we have in StringUtil/message_mapper, you might be able to use a similar design.

So using cpp::array?

I've switched over to using cpp::array.

michaelrj-google · 2025-03-26T20:37:32Z

libc/startup/uefi/CMakeLists.txt

@@ -0,0 +1,48 @@
+function(add_startup_object name)


I apparently missed that each of the targets currently has their own definition of this. Please add a todo referencing #133156 (the bug I've filed for this)

michaelrj-google · 2025-03-26T20:40:51Z

libc/src/__support/OSUtil/uefi/exit.cpp

@@ -7,14 +7,15 @@
 //===-----------------------------------------------------------------===//

 #include "src/__support/OSUtil/exit.h"
-#include "include/Uefi.h"
+#include "Uefi.h"


Including the public interface headers is bad practice inside of libc. If you need a type/macro it should go through a proxy header (e.g. https://github.com/llvm/llvm-project/blob/51bceb46f8eeb7c3d060387be315ca41855933c2/libc/hdr/types/struct_epoll_event.h)
If you need a function then it's recommended to create an internal interface for that function and call that both in the entrypoint and externally (e.g.

llvm-project/libc/src/__support/str_to_integer.h

Line 83 in 51bceb4

strtointeger(const char *__restrict src, int base,

)

Here and elsewhere in this patch.

Fixed, I just needed the EFI_SYSTEM_TABLE's stuff.

github-actions · 2025-03-27T02:51:59Z

✅ With the latest revision this PR passed the C/C++ code formatter.

petrhosek · 2025-03-27T17:29:39Z

libc/include/Uefi.yaml

+  - object_name: __llvm_libc_efi_system_table
    object_type: EFI_SYSTEM_TABLE *
-  - object_name: efi_image_handle
+  - object_name: __llvm_libc_efi_image_handle
    object_type: EFI_HANDLE


I don't think we should be adding LLVM libc internal symbols to standard headers like Uefi.h, rather these symbols should be declared in an internal header.

Also this renaming could be done in a separate PR since it's orthogonal to introducing crt1.

I don't think we should be adding LLVM libc internal symbols to standard headers like Uefi.h, rather these symbols should be declared in an internal header.

It's required to expose those symbols so programs can access UEFI specific things that are out of the scope of the libc

What's the intended usage for this? Is there a design doc I could read? That would make figuring out if this is the right approach a lot easier.

It's intended usage is any UEFI thing that isn't covered by a libc (see UEFI spec for exact details). This would cover things like interacting with the PCI bus or USB interfaces to graphics and networking. I don't have a design doc nor am I aware of one. You need to be able to access the system table to get access to the UEFI environment itself.

If there isn't a design doc, you should make one. It may take more time now, but writing out the plan for your interface will save time in the long run.

Alright and it'll cover #132150 (review) as well? I've never written a design doc before.

Yeah, one design doc would cover that as well. Don't stress too much about the initial design doc, part of the reason to create it is to get feedback and improve it.

The main thing to do is explain

What is your goal (e.g. provide a UEFI API that can do X/Y/Z)

How do you propose reaching that goal (e.g. Provide A/B/C libc functions)

Why do you think this design is a good solution (e.g. What properties does it have that are useful, what alternatives did you consider)

Here's an example of a design doc I wrote last year: https://discourse.llvm.org/t/rfc-project-hand-in-hand-llvm-libc-libc-code-sharing/77701
Feel free to copy the formatting, though you can definitely skip the "work estimates" section. That came from a template I was using and didn't end up being relevant.

The UEFI spec doesn't specify how to retrieve EFI_SYSTEM_TABLE and EFI_HANDLE (other than the handoff protocol), all UEFI entry points take it as an explicit argument. These symbols are a non-standard extension to spec and as such we need to be extra careful. For example, I don't think we should place these symbols in Uefi.h since that's less portable than having a custom header (which also let's you use __has_include). There are other questions, like for example whether these should be symbols or functions (the latter having an advantage of being interposable). All of these aspects should ideally be covered in the RFC.

michaelrj-google

I'd like to see a written down design before moving forward with this. That way we can agree on the direction. An RFC would be a good way to share the design.

michaelrj-google · 2025-04-01T17:04:06Z

libc/src/__support/OSUtil/uefi/error.h

+#include <errno.h>
+#include <limits.h>


We want to avoid including public libc headers directly. Also the internal limits is templated so you can just use cpp::numeric_limits<EFI_STATUS>::max() to get the maximum.

Suggested change

#include <errno.h>

#include <limits.h>

#include "hdr/errno_macros.h"

#include "src/__support/CPP/limits.h"

Using cpp::numeric_limits causes me to run into this issue:

error: constexpr variable 'UEFI_STATUS_ERRNO_MAP' must be initialized by a constant expression

Replaced constexpr with const and got error: shift count >= width of type:

#define EFI_ERROR_MAX_BIT (1 << (cpp::numeric_limits<EFI_STATUS>::max() - 1))

I guess EFI_ERROR_MAX_BIT as cpp::numeric_limits<EFI_STATUS>::max() is what I should be doing

Ok, this has been done.

michaelrj-google · 2025-04-01T17:04:46Z

libc/src/__support/OSUtil/uefi/error.h

+    {EFI_ENCODE_WARNING(EFI_WARN_RESET_REQUIRED), EINTR},
+}};
+
+static inline int uefi_status_to_errno(EFI_STATUS status) {


these should be LIBC_INLINE not static inline

michaelrj-google · 2025-04-01T17:07:09Z

libc/include/Uefi.yaml

+  - object_name: __llvm_libc_efi_system_table
    object_type: EFI_SYSTEM_TABLE *
-  - object_name: efi_image_handle
+  - object_name: __llvm_libc_efi_image_handle
    object_type: EFI_HANDLE


What's the intended usage for this? Is there a design doc I could read? That would make figuring out if this is the right approach a lot easier.

michaelrj-google · 2025-04-01T17:10:17Z

libc/startup/uefi/crt1.cpp

@@ -0,0 +1,30 @@
+//===-- Implementation of crt for UEFI ------------------------------------===//


This also needs a test, see https://github.com/llvm/llvm-project/blob/main/libc/test/integration/startup/linux/main_without_args.cpp for an example

lntue · 2025-04-24T17:44:53Z

libc/src/__support/OSUtil/uefi/error.h

@@ -28,7 +28,7 @@ struct UefiStatusErrnoEntry {
  int errno_value;
 };

-static constexpr cpp::array<UefiStatusErrnoEntry, 43> UEFI_STATUS_ERRNO_MAP = {{
+static const cpp::array<UefiStatusErrnoEntry, 43> UEFI_STATUS_ERRNO_MAP = {{


why is this not constexpr?

I believe it was causing a compiler error. I have about 2,370 builds to go until I can try.

Ok, it turns out clang likes it now. 🤷

RossComputerGuy · 2025-04-24T17:47:59Z

If at all possible, #131376 should likely come before this one so we can verify before merging whether this PR works instead of just verifying with a local build.

RossComputerGuy · 2025-04-24T19:48:53Z

Changed stuff around so now the system table and image handle are passed via AppProperties. Imo, no longer blocked on the UEFI RFC.

RossComputerGuy · 2025-05-01T01:10:17Z

Merged main in after #131376 to see how this PR and CI cooperate.

michaelrj-google · 2025-05-05T20:52:08Z

libc/startup/uefi/crt1.cpp

+//
+//===----------------------------------------------------------------------===//
+
+#include "config/uefi/app.h"


I think these should be config/app.h. There's no reason to have the extra layer of indirection if it's not being used. Here and other files.

michaelrj-google · 2025-05-05T20:53:38Z

libc/include/CMakeLists.txt

@@ -741,7 +741,6 @@ add_header_macro(
 add_header_macro(
  uefi
  ../libc/include/Uefi.yaml
-  Uefi.h.def


if you're removing the usage of Uefi.h.def you should also remove the file.

I don't remember this change... Not sure why the line was removed.

Oh now I remember, removed the file now.

michaelrj-google · 2025-05-05T20:55:30Z

libc/src/__support/OSUtil/uefi/error.h

+#define EFI_ERROR_MAX_BIT (cpp::numeric_limits<EFI_STATUS>::max())
+#define EFI_ENCODE_ERROR(value)                                                \
+  (EFI_ERROR_MAX_BIT | (EFI_ERROR_MAX_BIT >> 2) | (value))
+#define EFI_ENCODE_WARNING(value) ((EFI_ERROR_MAX_BIT >> 2) | (value))


these should be constexpr functions instead of macros.

michaelrj-google · 2025-05-05T20:57:09Z

libc/src/__support/OSUtil/uefi/error.h

+  // Unknown type
+  __builtin_unreachable();


this should probably be given a defined value. These functions shouldn't be called with values out of range, but if it does happen it'll be easier to debug.

I feel like we should trap here and the same for the function below for safety if possible.

michaelrj-google

LGTM

[libc][uefi] add crt1

b12c594

llvmbot added the libc label Mar 20, 2025

petrhosek reviewed Mar 20, 2025

View reviewed changes

[libc][uefi] drop mingw compat in crt0

dee779f

jhuber6 approved these changes Mar 21, 2025

View reviewed changes

overmighty reviewed Mar 21, 2025

View reviewed changes

petrhosek reviewed Mar 21, 2025

View reviewed changes

RossComputerGuy added 4 commits March 21, 2025 11:01

[libc][uefi] update efi symbols in efi main

7428fe0

[libc][uefi] add status codes

bf566bb

[libc][uefi] fix Uefi.h install name

40a26a0

[libc][uefi] add errno and uefi status conversion

e5a4692

petrhosek reviewed Mar 22, 2025

View reviewed changes

RossComputerGuy added 2 commits March 21, 2025 21:09

[libc][uefi] fix error, exit, and io OSUtil support compilation

d963ce2

[libc][uefi] get error codes working

4ccc169

michaelrj-google reviewed Mar 26, 2025

View reviewed changes

[libc][uefi] fix review

65da577

[libc][uefi] fix formatting

553ec1b

petrhosek reviewed Mar 27, 2025

View reviewed changes

[libc][uefi] use cpp::array for the error map

6648c0c

michaelrj-google reviewed Apr 1, 2025

View reviewed changes

RossComputerGuy added 3 commits April 1, 2025 18:52

[libc][uefi] use LIBC_INLINE in error

0596ada

[libc][uefi] add startup integration test

f87345a

[libc][uefi] use cpp::numeric_limits for UEFI_STATUS_ERRNO_MAP

0a47352

lntue reviewed Apr 24, 2025

View reviewed changes

RossComputerGuy added 2 commits April 24, 2025 12:36

[libc][uefi] move uefi system table & image to app config

ae46dfc

[libc] fix formatting

5e4c64f

RossComputerGuy added 2 commits April 24, 2025 12:49

[libc][uefi] use constexpr for errno map

74dd600

Merge branch 'main' into feat/libc-uefi/start

3aaccf0

michaelrj-google reviewed May 5, 2025

View reviewed changes

[libc][uefi] review adjustments

b5dbce3

michaelrj-google approved these changes May 8, 2025

View reviewed changes

RossComputerGuy merged commit 865fb9c into llvm:main May 9, 2025
16 checks passed

RossComputerGuy deleted the feat/libc-uefi/start branch May 9, 2025 16:02

		extern "C" InitCallback *__CTOR_LIST__[];
		extern "C" FiniCallback *__DTOR_LIST__[];

		EFI_HANDLE efi_image_handle;
		EFI_SYSTEM_TABLE *efi_system_table;


		main(0, NULL, NULL);

		// TODO: convert the return value of main to EFI_STATUS

	merge_relocatable_object(
	crt1
	.${LIBC_TARGET_ARCHITECTURE}.start
	.${LIBC_TARGET_ARCHITECTURE}.tls
	.do_start
	)

		@@ -0,0 +1,96 @@
		//===---------- UEFI implementation of error utils ------------- C++ --===//

		@@ -0,0 +1,30 @@
		//===-- Implementation of crt for UEFI ------------------------------------===//

[libc][uefi] add crt1 #132150

[libc][uefi] add crt1 #132150

Conversation

RossComputerGuy commented Mar 20, 2025

llvmbot commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhuber6 left a comment

Choose a reason for hiding this comment

RossComputerGuy commented Mar 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Mar 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Mar 22, 2025

michaelrj-google left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelrj-google left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Apr 24, 2025 • edited Loading

RossComputerGuy commented Apr 24, 2025

RossComputerGuy commented May 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 27, 2025 •

edited

Loading

RossComputerGuy commented Apr 24, 2025 •

edited

Loading