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

Add support for XCOFF #635

Merged
merged 3 commits into from
Jan 6, 2023
Merged

Add support for XCOFF #635

merged 3 commits into from
Jan 6, 2023

Conversation

bzEq
Copy link
Contributor

@bzEq bzEq commented Jan 6, 2023

XCOFF has different names for debug sections. This PR adds support for XCOFF which is the default object file format on AIX. Tests on AIX server all pass.

rg  '(Running)|(passed)' test.log 
2:     Running unittests src/lib.rs (target/debug/deps/gimli-f009eada21901b94)
464:test result: ok. 458 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.32s
466:     Running tests/convert_self.rs (target/debug/deps/convert_self-6975f7e844e5c08a)
472:test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.04s
474:     Running tests/parse_self.rs (target/debug/deps/parse_self-86a99c3c60cd3793)
488:test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.62s
490:     Running benches/bench.rs (target/debug/deps/bench-74d0874835d90a7c)
517:test result: ok. 23 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.88s
519:     Running unittests examples/dwarf-validate.rs (target/debug/examples/dwarf_validate-9cebfaaf145a12ef)
523:test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
525:     Running unittests examples/dwarfdump.rs (target/debug/examples/dwarfdump-dfec48f8bc4e68ab)
529:test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
531:     Running unittests examples/simple.rs (target/debug/examples/simple-bfa04b1b769853fc)
535:test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
537:     Running unittests examples/simple_line.rs (target/debug/examples/simple_line-826c47cfd577f13f)
541:test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

src/common.rs Outdated Show resolved Hide resolved
src/common.rs Outdated Show resolved Hide resolved
@bzEq
Copy link
Contributor Author

bzEq commented Jan 6, 2023

@philipc Thanks for your quick review. Are there any new tests should be added for XCOFF? I know there are some generated files under fixtures/self which are for tests/*_self.rs, are there any specific requirements or constraint if I have to add test files for XCOFF?

Also, since this part of code is to make addr2line work, I might send another PR in addr2line to make it work for XCOFF.

@bzEq bzEq requested a review from philipc January 6, 2023 10:36
@philipc
Copy link
Collaborator

philipc commented Jan 6, 2023

We don't have file format specific tests currently, and don't really need to because gimli is meant to be independent of the object file format. We do test the dwarfdump example on linux and macos executables in github actions, but I doubt there is a way to do the same for AIX. I don't think there is a need to add more test fixtures though.

Before sending a PR for addr2line, it'd be better to get the dwarfdump example working in this crate too. Maybe it did with your original changes, but it won't now. We'll need to choose section names based on the file format.

Copy link
Collaborator

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Thanks!

@philipc philipc merged commit 2a0afda into gimli-rs:master Jan 6, 2023
@bzEq bzEq deleted the xcoff branch January 6, 2023 11:02
@philipc
Copy link
Collaborator

philipc commented Jan 6, 2023

While I'm fine with adding xcoff_name as in this PR, I'm not sure that it's actually what we want to use in addr2line and dwarfdump. It would be better if the object crate handled this kind of logic, and Object::section_by_name currently already does that for Mach-O. I think it would be okay to change section_by_name for XCOFF too by doing string comparisons (and document this). Ideally we'd have something like Object::dwarf_section_by_id, but the problem with that is getting a common ID definition from somewhere, because gimli can't depend on object and vice versa.

@philipc
Copy link
Collaborator

philipc commented Jan 6, 2023

I think xcoff_name would be suitable for use in backtrace-rs.

@bzEq
Copy link
Contributor Author

bzEq commented Jan 6, 2023

Object::section_by_name currently already does that for Mach-O.

Thanks for this info, that sounds more smooth if get the work done in object crate. I'll take a look at this code path. If I find that's viable for XCOFF too, reverting this PR is fine to me, since it becomes dead code then :).

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.

None yet

2 participants