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.

Sign up for GitHub

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

Jump to bottom

[llvm-readelf] Add --extra-sym-info #65580

Merged
merged 2 commits into from
Sep 11, 2023
Merged

[llvm-readelf] Add --extra-sym-info #65580

merged 2 commits into from
Sep 11, 2023

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Sep 7, 2023

GNU readelf introduced --extra-sym-info/-X to display the section name
for --syms (https://sourceware.org/PR30684). Port the feature, which is
currently llvm-readelf only.

For STO_AARCH64_VARIANT_PCS/STO_RISCV_VARIANT_PCS, the Ndx and Name
columns may not be aligned.

@MaskRay MaskRay requested a review from jh7370 September 7, 2023 08:14
jh7370
jh7370 reviewed Sep 8, 2023
View reviewed changes
Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

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

No major comments, all looks generally reasonable to me.

I noticed this option extends the column headers, but we're already printing the extra information without the option. Is that the same as GNU, or are we currently printing more than them?

llvm/test/tools/llvm-readobj/ELF/symbols.test Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/symbols.test Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test Show resolved Hide resolved
llvm/tools/llvm-readobj/ELFDumper.cpp Show resolved Hide resolved
jh7370
jh7370 approved these changes Sep 11, 2023
View reviewed changes
Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

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

LGTM! Maybe a release note too?

llvm/test/tools/llvm-readobj/ELF/symbols.test Show resolved Hide resolved
@MaskRay
[llvm-readelf] Add --extra-sym-info
f6a56ae 
GNU readelf introduced --extra-sym-info/-X to display the section name
for --syms (https://sourceware.org/PR30684). Port the feature, which is
currently llvm-readelf only.

For STO_AARCH64_VARIANT_PCS/STO_RISCV_VARIANT_PCS, the Ndx and Name
columns may not be aligned.
@MaskRay MaskRay merged commit eb81493 into llvm:main Sep 11, 2023
2 of 3 checks passed
@MaskRay MaskRay deleted the readobj-X branch September 11, 2023 18:09
@michaelrj-google michaelrj-google mentioned this pull request Sep 12, 2023
Merged
@vzakhari vzakhari mentioned this pull request Sep 12, 2023
Closed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
@MaskRay @ZijunZhaoCCK
[llvm-readelf] Add --extra-sym-info (llvm#65580)
8bc682b 
GNU readelf introduced --extra-sym-info/-X to display the section name
for --syms (https://sourceware.org/PR30684). Port the feature, which is
currently llvm-readelf only.

For STO_AARCH64_VARIANT_PCS/STO_RISCV_VARIANT_PCS, the Ndx and Name
columns may not be aligned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jh7370 jh7370 jh7370 approved these changes

Assignees
No one assigned
Labels
tools:llvm-readobj/readelf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MaskRay @jh7370