-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[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
[libc][uefi] add crt1 #132150
Changes from 11 commits
b12c594
dee779f
7428fe0
bf566bb
40a26a0
e5a4692
d963ce2
4ccc169
65da577
553ec1b
6648c0c
0596ada
f87345a
0a47352
ae46dfc
5e4c64f
74dd600
3aaccf0
b5dbce3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ types: | |
| enums: [] | ||
| functions: [] | ||
| objects: | ||
| - object_name: efi_system_table | ||
| - 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 | ||
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,98 @@ | ||||||||||
| //===----------- UEFI implementation of error utils --------------*- C++-*-===// | ||||||||||
| // | ||||||||||
| // 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 | ||||||||||
| // | ||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||
|
|
||||||||||
| #ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_UEFI_ERROR_H | ||||||||||
| #define LLVM_LIBC_SRC___SUPPORT_OSUTIL_UEFI_ERROR_H | ||||||||||
|
|
||||||||||
| #include "include/llvm-libc-types/EFI_STATUS.h" | ||||||||||
| #include "src/__support/CPP/array.h" | ||||||||||
| #include "src/__support/macros/config.h" | ||||||||||
| #include <errno.h> | ||||||||||
| #include <limits.h> | ||||||||||
|
||||||||||
| #include <errno.h> | |
| #include <limits.h> | |
| #include "hdr/errno_macros.h" | |
| #include "src/__support/CPP/limits.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess EFI_ERROR_MAX_BIT as cpp::numeric_limits<EFI_STATUS>::max() is what I should be doing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this has been done.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be LIBC_INLINE not static inline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we should trap here and the same for the function below for safety if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||||||||||
| # TODO: Use generic "add_startup_object" https://github.com/llvm/llvm-project/issues/133156 | ||||||||||||||
| function(add_startup_object name) | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||||
| DEPENDS | ||||||||||||||
| libc.src.__support.OSUtil.uefi.uefi_util | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 llvm-project/libc/startup/linux/CMakeLists.txt Lines 114 to 119 in 3840f78
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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha, yeah I think for the purposes of this platform and the capabilities currently in |
||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| 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() | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| //===-- Implementation of crt for UEFI ------------------------------------===// | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| // | ||
| // 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/OSUtil/uefi/error.h" | ||
| #include "src/__support/macros/config.h" | ||
|
|
||
| extern "C" { | ||
| EFI_HANDLE __llvm_libc_efi_image_handle; | ||
| EFI_SYSTEM_TABLE *__llvm_libc_efi_system_table; | ||
|
|
||
| int main(int argc, char **argv, char **envp); | ||
|
|
||
| EFI_STATUS EfiMain(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) { | ||
| __llvm_libc_efi_image_handle = ImageHandle; | ||
| __llvm_libc_efi_system_table = SystemTable; | ||
|
|
||
| // TODO: we need the EFI_SHELL_PROTOCOL, malloc, free, and UTF16 -> UTF8 | ||
| // conversion. | ||
| return LIBC_NAMESPACE::errno_to_uefi_status(main(0, nullptr, nullptr)); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you're removing the usage of
Uefi.h.defyou should also remove the file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember this change... Not sure why the line was removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh now I remember, removed the file now.