This repository has been archived by the owner on Jul 1, 2023. It is now read-only.
Fix update-debug-sections for AArch64 #255
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
ElvinaYakubova
force-pushed
the
debug-sections
branch
from
31de4a8
to
10cc215
Compare
|
@ElvinaYakubova, thank you for the fix. Could you please add a test case? |
rafaelauler
reviewed
Nov 23, 2021
Sure, will do |
rafaelauler
force-pushed
the
main
branch
2 times, most recently
from
97cb228
to
df64e82
Compare
ElvinaYakubova
force-pushed
the
debug-sections
branch
from
10cc215
to
b2b7dc0
Compare
|
Thanks for the review @rafaelauler, fixed as you suggested. I've added tests and also noticed when trying to import line-number.test, that "#debug line" number doesn't always show the correct value, so I'm trying to fix that as well. |
rafaelauler
reviewed
Dec 13, 2021
Summary: Use injected functions with fixed addresses to patch original function entries. (cherry picked from FBD24133890)
Summary: At the moment we are not using PatchEntries pass in non-relocation mode on ELF. However, we will use it on MachO. (cherry picked from FBD24235271)
Summary: Add support for emitting code into a new segment on MachO (in the instrumentation mode). (cherry picked from FBD24097547)
Summary: Append ".cold.0" suffix to the original part of the name, such that "foo/1" becomes "foo.cold.0/1" instead of "foo/1.cold.0". (cherry picked from FBD24246112)
Summary: _end is "defined" but its address doesn't belong to any section. This diff adds special handling for this symbol. (cherry picked from FBD24249120)
Summary: On targets that support it, emit size of the emitted function symbol. At the moment there's no use for the size except that it is visible in a temporary .o file symbol table. (cherry picked from FBD24246177)
Summary: Add first bits to support emitting instrumented code on MachO. This diff enables us to instrument branches / emit counters. (cherry picked from FBD24255164)
Summary: Emit line info for functions that were not emitted in relocation mode. (cherry picked from FBD24267650)
Summary: Match BinaryFunction to a DWARFUnit based on the unit's address ranges skipping the parsing of DIEs. (cherry picked from FBD24269325)
Summary: When placing restore instructions in the shrink wrapping pass, we typically put them right before the last instruction of a block at the dominance frontier. If this instruction happened to have a prefix, because the MC lib separates prefix into separate MCInsts, we would accidentally put a load between a prefix and another instruction. Fix this. (cherry picked from FBD24295324)
Summary: Add first bits to cross-compile the runtime for OSX. (cherry picked from FBD24330977)
Summary: Do not store processed DWARF DIEs, but instead process them while reading one at a time. Reduces memory consumption when updating debug info by 10%-25%. (cherry picked from FBD24327029)
Summary: This diff is a preparation for loading the runtime on MachO. The proposed schema is the following: 1/ Function "bolt_instr_setup" is placed into the predefined section "setup" (in the final setting this function will be coming from the instrumentation runtime but we still will be placing it into this section). 2/ In the instrumentation pass we create a symbol "bolt_instr_setup" and inject the corresponding call into the beginning of the function representing the entry point of the binary. (cherry picked from FBD24329530)
Summary: When -hot-text is on, do not read __hot_start and __hot_end from input (inserted by a linker script with the intent of ordering functions). This can confuse BOLT into creating a function with this name depending on which address the symbol lands and we will assert when trying to emit our own __hot_start/__hot_end with symbol redefinition. (cherry picked from FBD24366636)
Summary: When optimizing input with relocations, make it faster and less memory-hungry with lite mode. (cherry picked from FBD24374241)
Summary: While refactoring the pass, I removed the important transactional property of the patching process. Restore it. (cherry picked from FBD24440214)
Summary: Change .dot dumps filename format from <function>-<passname>.dot to <function>-<passidx>_<passname>.dot This change helps navigate dumps by making the pass order explicit. Example: execute_stack_op.cold.6-1(*2)-00_build-cfg.dot execute_stack_op.cold.6-1(*2)-01_validate-internal-calls.dot execute_stack_op.cold.6-1(*2)-02_strip-rep-ret.dot ... (cherry picked from FBD24452903)
Summary: Some symbols in .dynsym will be erroneously marked as belonging to a non-allocatable section that BOLT can remove. In that case, keep the original invalid index for such symbols instead of setting the UNDEF index. (cherry picked from FBD24488677)
Summary: Only use dump() method under DEBUG() macro. (cherry picked from FBD24666481)
Summary: TailDuplication::isInCacheLine makes the assumption that the block has a valid layout index, which is not the case for unreachable blocks. Add a check for a valid layout index. (cherry picked from FBD32659755)
Summary: The intent is clearly to check the current basic block. (cherry picked from FBD32658103)
Summary: Print the relocation name instead of just the number. (cherry picked from FBD32704832)
Summary: The debug message for the last fall-through block was printed under the reverse condition, i.e. when the block was not a fall-through. Remove the debug message. If we'll need such information, we can add a pass with more analysis, i.e. checking the last instruction, if the block is reachable, etc. (cherry picked from FBD32670816)
… a workaround Summary: Disable const/copy propagation as a bug workaround. Also add the debug logging in aggressive duplication. (cherry picked from FBD32774744)
Summary: Remove unused code identified via coverage report. (cherry picked from FBD32818329)
Summary: Remove unused code identified via coverage report. (cherry picked from FBD32818608)
Summary: Remove the test and its inputs. (cherry picked from FBD32855788)
Summary: Some optimizations may remove all instructions in a basic block. The pass will cleanup the CFG afterwards by removing empty basic blocks and merging duplicate CFG edges. The normalized CFG is printed under '-print-normalized' option. (cherry picked from FBD32774360)
Summary: Currently, RuntimeDyld will not allocate a section without relocations even if such a section is marked allocatable and defines symbols. When we emit .debug_line for compile units with unchanged code, we output original (input) data, without relocations. If all units are emitted in this way, we will have no relocations in the emitted .debug_line. RuntimeDyld will not allocate the section and as a result we will write an empty .debug_line section. To workaround the issue, always emit a relocation of RELOC_NONE type when emitting raw contents to debug_line. (cherry picked from FBD32909869)
Summary: Make explicit the contact of component owners. (cherry picked from FBD32904304)
Summary: For DWP case the AbbreviationsOffset is the offset of the abbrev contribution in the DWP file, so can be none zero. (cherry picked from FBD32961240)
Summary: Remove the copyright/license message for the code originated from Facebook. (cherry picked from FBD32998404)
ElvinaYakubova
force-pushed
the
debug-sections
branch
2 times, most recently
from
8e53a10
to
58ed4dd
Compare
|
Thanks Elvina! Sorry, I tried to import this but the linter is complaining about formatting of the changes in two other files as well (Relocation.cpp, DWARFRewriter.cpp). Could you run clang-format in the changes introduced there too? |
Summary: This patch adds AArch64 relocations handling in case updating of debug sections is enabled Elvina Yakubova, Advanced Software Technology Lab, Huawei
ElvinaYakubova
force-pushed
the
debug-sections
branch
from
58ed4dd
to
dd83f2e
Compare
|
@rafaelauler Sorry for the inconvenience, I fixed it, now everything should be ok. |
aaupov
force-pushed
the
main
branch
3 times, most recently
from
fe64c40
to
5e01a26
Compare
|
Committed: 04b81f7. |
rafaelauler
added a commit
to llvm/llvm-project
that referenced
this pull request
May 11, 2023
facebookarchive/BOLT#255 accidentally omitted a relocation type when refactoring the code. Add this type back and change function name so its intent is more clear. Reviewed By: #bolt, Amir Differential Revision: https://reviews.llvm.org/D150335
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds AArch64 relocations handling in case updating of debug sections is enabled.
Refers to #253 problem that occurs when update-debug-sections flag is used. Previously, in the updateLineTableOffsets function, only X86 relocation (number 10) was created, and further, that number was passed to the getSizeForType function, which is architecture-dependent, so it tried to find AArch64 relocation with the same number, and it was R_AARCH64_P32_ADR_PREL_LO21.
Elvina Yakubova,
Advanced Software Technology Lab, Huawei