Skip to content

added: z-undefs flag with test#1030

Merged
davidlattimore merged 1 commit into
wild-linker:mainfrom
AadiWaghray:z-undefs
Aug 10, 2025
Merged

added: z-undefs flag with test#1030
davidlattimore merged 1 commit into
wild-linker:mainfrom
AadiWaghray:z-undefs

Conversation

@AadiWaghray
Copy link
Copy Markdown
Contributor

add undefs option for z flag following further discussion in issue #810. PR is recreated after rebase issue on previous PR (#865). The "other two" issues mentioned in previous PR persist. I want to confirm how to proceed.

@davidlattimore
Copy link
Copy Markdown
Member

From the test failures, it looks like you need to add a couple of DiffIgnores. e.g. //#DiffIgnore:.dynamic.DT_RELA.

Don't worry about running into git issues. From the look of things, you probably merged to head and there had just been a lot of commits since you did the original version of the PR. Different people have different workflows. Personally, I always rebase and amend commits. Some people prefer to merge and create fixup commits. Either is fine and we can always squash and rebase when we merge the PR. The git book is a good resource to learn more - https://git-scm.com/book/en/v2. The jj version control system is also good. That's what I've been using for the last year or so.

Comment thread wild/tests/sources/z-undefs.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wild includes integration tests, and the current CI failure is caused by these tests. The integration tests compare linking results with other linkers (specifically GNU ld in this case) and generate errors if any discrepancies are found.
However, not all differences need to be resolved—you can explicitly mark certain parts as acceptable to ignore differences without triggering errors.
Examining the CI output (or by running cargo test locally) reveals that two sections differ: .dynamic.DT_RELA and .dynamic.DT_RELAENT. These differences can be safely ignored. To suppress these differences, you can add lines like //#DiffIgnore:.dynamic.DT_RELA or //#DiffIgnore:.dynamic.DT_RELAENT to the test source files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added these //#DiffIgnore: statements along with the //#Config: statements to allow for both tests to be done from a single file. However, in local testing, the z-undefs config fails stating the "Binary ran for too long." This occurs after adding the //#DiffIgnore: statements to the z-undefs config. Is there a way you recommend tracing this problem?

Copy link
Copy Markdown
Member

@lapla-cogito lapla-cogito Aug 10, 2025

Choose a reason for hiding this comment

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

You can set //#RunEnabled:false to disable executing the binary, which will make the test pass.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you. That solved the issue. Was the problem that the linker produced a bad binary by ignoring the undefined symbol error, so when it tried to run the binary, it wasn't able to? If that's the case, I'm sorry for not realizing it earlier.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That might be a problem too, but the main problem is that the binary isn't an executable, it's a shared object, which can't be directly executed. When we get Mode:dynamic, we should probably default to RunEnabled:false.

Comment thread wild/tests/integration_tests.rs Outdated
fixed: header arguments for test

revised: z-undefs/ z-defs tests to single file

fixed: test error trying to run bad binary
@AadiWaghray
Copy link
Copy Markdown
Contributor Author

I appreciate the leniency and advice. I'll be sure to review the git book.

@AadiWaghray AadiWaghray force-pushed the z-undefs branch 2 times, most recently from 64b74b1 to 74e8662 Compare August 10, 2025 22:21
@davidlattimore davidlattimore merged commit 6dc3790 into wild-linker:main Aug 10, 2025
23 checks passed
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