Skip to content
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

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

ADKaster
Copy link
Member

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

Copy link
Collaborator

@awesomekling awesomekling left a 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;
Copy link
Collaborator

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.

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, also removed parameter names

Kernel/init.cpp Outdated
@@ -192,6 +192,38 @@ extern "C" {
multiboot_info_t* multiboot_info_ptr;
}

// todo remove
Copy link
Collaborator

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

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 :)

Copy link
Member Author

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

Choose a reason for hiding this comment

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

Typo: somthing -> something

Copy link
Member Author

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
```
@awesomekling awesomekling merged commit 233ea7e into SerenityOS:master Oct 31, 2019
@awesomekling
Copy link
Collaborator

Awesome! Thank you for fixing this :)

@bugaevc
Copy link
Member

bugaevc commented Oct 31, 2019

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:

Lockable<HashMap<u16, UDPSocket*>>& UDPSocket::sockets_by_port()
{
static Lockable<HashMap<u16, UDPSocket*>>* s_map;
if (!s_map)
s_map = new Lockable<HashMap<u16, UDPSocket*>>;
return *s_map;
}

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 lazy_static macro to make this both convenient and lazy.

@ADKaster ADKaster deleted the AKaster/GlobalCtors branch November 1, 2019 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants