search

facebookarchive / BOLT

Binary Optimization and Layout Tool - A linux command-line utility used for optimizing performance of binaries
2.51k stars 176 forks source link

Dwarf: Fix high pc patching #267

Closed yota9 closed 2 years ago

yota9 commented 2 years ago

The DW_FORM_addr form of highPC address is written in absolute addres, the data form is written in offset-from-low pc format.

Vladislav Khmelevsky, Advanced Software Technology Lab, Huawei

yota9 commented 2 years ago

The C compilers are usually produces dwarf in with DW_FORM_data8 DW_AT_high_pc. The dwarfdump shows that the value is relative:

                      DW_AT_low_pc                0x00400100
                      DW_AT_high_pc               <offset-from-lowpc>218

But the golang compiler produces high pc in DW_FORM_addr adress

                      DW_AT_low_pc                0x00457056
                      DW_AT_high_pc               0x004572e3

In this case we must not subtract low_pc value from high_pc.

ayermolo commented 2 years ago

@yota9 Sorry for late reply. Can you rebase on latest. I redid how we handle .debug_info. It is now being re-written. So we are no longer constrained by replacement fields being the same size.

yota9 commented 2 years ago

Hello @ayermolo ! Thank you for your reply. I've rebased both the patches

ayermolo commented 2 years ago

Would it be possible to add a test?

yota9 commented 2 years ago

Hello! For the both patches I would like to add compiled golang binary executable (actually 2, for both aarch64 and x86) where the problems were met. The thing is that the size of the binary is around 5mb each and 18mb in yaml format. The same problem would be when the golang support PR will be created, since I would like to add a few more tests, and I don't think we may add dependency on go compiler. Anyway as I understand the binary size is too large to just put them in test dir, do you have any suggestions on this? Adding @maksfb @rafaelauler @aaupov to conversation :) Maybe I need to PR them to the separate test repo the @rafaelauler own?

aaupov commented 2 years ago

Hello! For the both patches I would like to add compiled golang binary executable (actually 2, for both aarch64 and x86) where the problems were met. The thing is that the size of the binary is around 5mb each and 18mb in yaml format. The same problem would be when the golang support PR will be created, since I would like to add a few more tests, and I don't think we may add dependency on go compiler. Anyway as I understand the binary size is too large to just put them in test dir, do you have any suggestions on this? Adding @maksfb @rafaelauler @aaupov to conversation :) Maybe I need to PR them to the separate test repo the @rafaelauler own?

Sorry, I've missed this question. Yes, the current solution is to add such binary tests to rafaelauler/bolt-tests repo. Please start a PR in that repo.

yota9 commented 2 years ago

Hello @ayermolo ! I've created the test pr for both this and #268 problems. If both these reviews are good please tell me, I will create review in LLVM and then submit the patches there. Thank you!

ayermolo commented 2 years ago

Hello @ayermolo ! I've created the test pr for both this and #268 problems. If both these reviews are good please tell me, I will create review in LLVM and then submit the patches there. Thank you!

@yota9 Sorry for late reply. We are in the performance reviews time ATM. :) Does it make sense to just re-create those patches in llvm monorepo and continue review there?

yota9 commented 2 years ago

No problems @ayermolo :) Sure, let me do this tomorrow.

yota9 commented 2 years ago

Re-opened in https://reviews.llvm.org/D117217