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

Use llvm-objcopy on Linux x64 #48444

Merged
1 commit merged into from
Feb 18, 2021
Merged

Use llvm-objcopy on Linux x64 #48444

1 commit merged into from
Feb 18, 2021

Conversation

am11
Copy link
Member

@am11 am11 commented Feb 18, 2021

Consider the following commands sequence for symbol stripping:

COMMAND ${CMAKE_OBJCOPY} --only-keep-debug ${strip_source_file} ${strip_destination_file}
COMMAND ${CMAKE_OBJCOPY} --strip-unneeded ${strip_source_file}
COMMAND ${CMAKE_OBJCOPY} --add-gnu-debuglink=${strip_destination_file} ${strip_source_file}

Before and after line 291, the clang-compiled libcoreclr.so on Linux x64 has .init_array and .fini_array with ES value (sh_entsize; /* Entry size if section holds table */) set to 0 (zero):

# tested by adding the following line (uncommented) before and after line 291
# COMMAND eu-readelf -S ${strip_source_file} 

Section Headers:
[Nr] Name                 Type         Addr             Off      Size     ES Flags Lk Inf Al
...snip...
[20] .fini_array          FINI_ARRAY   000000000067cf30 0067bf30 00000040  0 WA     0   0  8
[21] .init_array          INIT_ARRAY   000000000067cf70 0067bf70 00000110  0 WA     0   0  8
...snip...

(the gcc-compiled version has ES value of 8 (eight) for these two symbols here)

After line 292, the clang-compiled libcoreclr.so gets the ES value changed to 8 (eight) but now the libcoreclr.so.dbg has the ES value 0 (zero) for those symbols. This is because we are using GNU's objcopy instead of LLVM's, and this causes a mismatch between symbols and binary.

This was found with symbol unstripping-then-stripping roundtrip test:

$ eu-unstrip -o libcoreclr.embedded_symbols.so \
    ~/.dotnet/shared/Microsoft.NETCore.App/6.0.0-preview.2.21105.12/libcoreclr.so \
    ~/.dotnet/shared/Microsoft.NETCore.App/6.0.0-preview.2.21105.12/libcoreclr.so.dbg
$ eu-strip --strip-debug libcoreclr.embedded_symbols.so \
    -f libcoreclr.roundtripped.so.dbg -o libcoreclr.roundtripped.so
$ colordiff -u \
    <(eu-readelf -S ~/.dotnet/shared/Microsoft.NETCore.App/6.0.0-preview.2.21105.12/libcoreclr.so) \
    <(eu-readelf -S libcoreclr.roundtripped.so)
$ colordiff -u \
    <(eu-readelf -S ~/.dotnet/shared/Microsoft.NETCore.App/6.0.0-preview.2.21105.12/libcoreclr.so.dbg) \
    <(eu-readelf -S libcoreclr.roundtripped.so.dbg)

This PR makes it so we use objcopy out of llvm-toolchain when compiling with clang for relevant x64 ELF targets, and continue to use GNU's objcopy for gcc. With this change, both symbols and binary have ES values 0 (zero) for the said symbols. With gcc, the value is 8 (eight) in both SO and DBG before and after this change (and before and after the strip roundtrip).

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@am11 am11 marked this pull request as ready for review February 18, 2021 12:01
@am11
Copy link
Member Author

am11 commented Feb 18, 2021

cc @janvorli

@janvorli
Copy link
Member

@am11 what effect does this discrepancy have on debugging?

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@ghost
Copy link

ghost commented Feb 18, 2021

Hello @janvorli!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@am11
Copy link
Member Author

am11 commented Feb 18, 2021

@janvorli, I am not aware of any direct effect on debugging; was embedding the symbols (with eu-unstrip) to be able to attach the remote debugger to a .NET unikernel; wanted to make sure if unstripping is safe / valid, so I did the roundtrip sanity test and found out the ES value difference. Someone in elfutils community mentioned that other minor differences (due to padding, swapping of sections etc.) are ignorable, but the difference in ES value is concerning. After that we found the difference in clang-compiled libcoreclr.so and .dbg. At which point I decided to fix it before it highlights some other issues.

Ideally, we should be using all tools of the same toolchain, but currently we have some older versions to support so it requires a lot of back and forth testing. I guess we can more conveniently align this in future, when we will deprecate older versions of cross toolchain.

@ghost ghost merged commit ba50840 into dotnet:master Feb 18, 2021
@am11 am11 deleted the feature/native/configurations branch February 19, 2021 03:44
@EgorBo EgorBo mentioned this pull request Mar 3, 2021
jkotas added a commit to jkotas/runtime that referenced this pull request Mar 3, 2021
jkotas added a commit to jkotas/runtime that referenced this pull request Mar 4, 2021
ghost pushed a commit that referenced this pull request Mar 4, 2021
jkotas added a commit that referenced this pull request Mar 4, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants