Skip to content

Conversation

@IAmTheNerdNextDoor
Copy link
Contributor

I was following these sections and kept having errors. After some experimenting, I found out there were some typos in the code examples of the changed files.

(For example, there was a ',' where there should have been a '.' in the linker script.

There was also a '%' where there should have been a '$' when calling variables in the Makefile. In the same Makefile, there was also a mistyping of 'x86_64-elf-ld' which had an '_' in between 'x86_64' and 'elf'.

I also fixed an issue that I encountered on my side with the same programs where it wouldn't link without -shared being specified in Makefile.)

I hope this pull request helps.

@dreamos82
Copy link
Member

Hey thanks for the PR, and thanks for spotting those errors and fixing it.

The only thing i'm not sure is about the -shared parameter, i don't think it should be necessary (in fact it is making the linking phase failing on my kernel).

@DeanoBurrito what do you think?

@AFellowSpeedrunner meanwhile feel free to add yourself in the contributors list.

@IAmTheNerdNextDoor
Copy link
Contributor Author

Hey thanks for the PR, and thanks for spotting those errors and fixing it.

The only thing i'm not sure is about the -shared parameter, i don't think it should be necessary (in fact it is making the linking phase failing on my kernel).

@DeanoBurrito what do you think?

@AFellowSpeedrunner meanwhile feel free to add yourself in the contributors list.

Ah, thank you.

I'll try and find the contributor list now.

I added shared in as I get this error upon compiling without it.

"x86_64-elf-ld: -f may not be used without -shared"

@dreamos82
Copy link
Member

Can you share the linking code?

@IAmTheNerdNextDoor
Copy link
Contributor Author

Assuming you mean the linking side of Makefile, here you go.

LD_FLAGS = -T link.ld -ffreestanding

all: $(OBJS)
	@echo "Linking program ..."
	$(LD) $(LD_FLAGS) $(OBJS) -o $(TARGET)
	@echo "Program linked, placed @ $(TARGET)"

My link file is named differently but it shouldn't be an issue.

@dreamos82
Copy link
Member

I think that could be another error on our side (apologies)
Feel free to fix that (since you already opened the pr), basically in the code example here: https://github.com/dreamportdev/Osdev-Notes/blob/master/01_Build_Process/03_Gnu_Makefiles.md#simple-makefile-example

on the #flags section, the -ffreestanding in the LD_FLAGS should be removed (so technically also your -shared) , since this is a gcc specific option!

In ld -f is doing something different.

So also on your kernel try to remove -ffreestanding -shared and it should work.

Is that correct @DeanoBurrito ?

@IAmTheNerdNextDoor
Copy link
Contributor Author

Builds successfully without -ffreestanding and -shared.

@IAmTheNerdNextDoor
Copy link
Contributor Author

Does this look good?

@dreamos82
Copy link
Member

yep looks good to me. Let's wait for @DeanoBurrito review :)

Thanks again! :)

@DeanoBurrito
Copy link
Member

on the #flags section, the -ffreestanding in the LD_FLAGS should be removed (so technically also your -shared) , since this is a gcc specific option!

Yeah thats correct, -ffreestanding shouldn't have been in the linker flag, it's only intended for the compiler. If you're using the compiler driver instead of calling the linker directly (e.g. linking with gcc and letting it execute ld for us) those compiler-specific flags get filtered out.

Thanks for the PR @AFellowSpeedrunner, I dont see any issues so I'll merge it.

@DeanoBurrito DeanoBurrito merged commit f38606d into dreamportdev:master Dec 28, 2024
@IAmTheNerdNextDoor
Copy link
Contributor Author

on the #flags section, the -ffreestanding in the LD_FLAGS should be removed (so technically also your -shared) , since this is a gcc specific option!

Yeah thats correct, -ffreestanding shouldn't have been in the linker flag, it's only intended for the compiler. If you're using the compiler driver instead of calling the linker directly (e.g. linking with gcc and letting it execute ld for us) those compiler-specific flags get filtered out.

Thanks for the PR @AFellowSpeedrunner, I dont see any issues so I'll merge it.

Thank you very much @DeanoBurrito and @dreamos82 this was very enjoyable for me!

I'm happy that I've helped with this. :)

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.

3 participants