-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Kernel: Add bare minimum for global constructors #707
Conversation
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.
Dude, awesome! :)
(void)f; | ||
(void)p; | ||
(void)d; | ||
return 0; |
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.
Let's put an ASSERT_NOT_REACHED();
in this function.
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, also removed parameter names
Kernel/init.cpp
Outdated
@@ -192,6 +192,38 @@ extern "C" { | |||
multiboot_info_t* multiboot_info_ptr; | |||
} | |||
|
|||
// todo remove |
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 you meant to remove this :)
Kernel/init.cpp
Outdated
for (ctor_func_t* ctor = &start_ctors; ctor < &end_ctors; ctor++) | ||
(*ctor)(); | ||
|
||
// todo remove |
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 you meant to remove this too :)
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.
right, had to test somehow
Kernel/init.cpp
Outdated
extern ctor_func_t end_ctors; | ||
|
||
// Define some Itanium C++ ABI methods to stop the linker from complaining | ||
// If we actually call these somthing has gone horribly wrong |
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.
Typo: somthing -> something
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.
spell checker failed me again, fixed
Add text.startup to the .text block, add .ctors as well. Use them in init.cpp to call global constructors after gtd and idt init. That way any funky constructors should be ok. Also defines some Itanium C++ ABI methods that probably shouldn't be, but without them the linker gets very angry. If the code ever actually tries to use __dso_handle or call __cxa_atexit, there's bigger problems with the kernel. Bit of a hack would be an understatement but hey. It works :) ``` year: 2019, month: 10, day: 31 PIC(i8259): cascading mode, vectors 0x50-0x58 Globalglobalglobalglobalglobal Global Struct values: 100 10 PS2MouseDevice: Device detected ```
89ef188
to
a7e2edf
Compare
Awesome! Thank you for fixing this :) |
This is cool, but we need to be very conscious about using this, because it will further increase the boot time. Currently, various global resources are lazily created like this: serenity/Kernel/Net/UDPSocket.cpp Lines 15 to 21 in 233ea7e
with global constructors, it could be simplified to just Lockable<HashMap<u16, UDPSocket*>> g_sockets_by_port; at the cost of running the constructor at bootup instead of on first access. Of course we don't have to change existing occurences of this pattern, but it's going to be too tempting to just do this in new code. I wonder if we could have something like the |
Add text.startup to the .text block, add .ctors as well.
Use them in init.cpp to call global constructors after
gtd and idt init. That way any funky constructors should be ok.
Also defines some Itanium C++ ABI methods that probably shouldn't be,
but without them the linker gets very angry.
If the code ever actually tries to use __dso_handle or call
__cxa_atexit, there's bigger problems with the kernel.
Bit of a hack would be an understatement but hey. It works :)