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

Ensure that object file section IDs are correct #1603

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Jan 9, 2025

I can't make a test case to demonstrate this because it doesn't currently occur, since RGBASM merges section fragments (and unions) at assembly time when they're in the same object:

	// Check if another section exists with the same name; merge if yes, otherwise create one

	Section *sect = sect_FindSectionByName(name);

	if (sect) {
		mergeSections(*sect, type, org, bank, alignment, alignOffset, mod);
	} else {
		sect = createSection(name, type, org, bank, alignment, alignOffset, mod);
	}

However, this is a theoretically correct fix which would avoid bugs if that ever changes.

@Rangi42 Rangi42 added bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM labels Jan 9, 2025
@Rangi42 Rangi42 added this to the 0.9.1 milestone Jan 9, 2025
@Rangi42
Copy link
Contributor Author

Rangi42 commented Jan 9, 2025

This change/fix was motivated by attempting to rebase #716 to the current master. Back when it was coded, section IDs were derived by searching through a linked list for a specific Section * pointer, which naturally distinguished between multiple fragments with the same name. The current master instead finds whichever section ID corresponds to the first fragment/union with that name. "Inline fragments"/"fragment literals" are one feature which expose this difference, but there could theoretically be others.

@Rangi42 Rangi42 marked this pull request as draft January 9, 2025 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant