Skip to content

[libc][uefi] add crt1 #132150

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 19 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions libc/startup/uefi/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
function(add_startup_object name)
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

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
Copy link
Member

Choose a reason for hiding this comment

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

Nit: formatting.

Suggested change
SRCS
crt1.cpp
SRCS
crt1.cpp

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done

)

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()
29 changes: 29 additions & 0 deletions libc/startup/uefi/crt1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//===-- 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
//
//===--------------------------------------------------------------------===//
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done


#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"

EFI_HANDLE efi_image_handle;
EFI_SYSTEM_TABLE *efi_system_table;
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I got it to build correctly with extern.


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;

main(0, NULL, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
main(0, NULL, NULL);
main(0, nullptr, nullptr);

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done


// TODO: convert the return value of main to EFI_STATUS
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's C standard errno values if nothing else.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

return 0; // TODO: EFI_SUCCESS
}
Loading