This commit changes all relocations to be relocated with symbols.
Without this commit, errors may occur in some cases, such as when using
`llc/lto+relax`, or combining relaxed and norelaxed object files using
`ld -r`.
Some tests updated.
In review of bbde6b, I had originally proposed that we support the
legacy text format. As review evolved, it bacame clear this had been a
bad idea (too much complexity), but in order to let that patch finally
move forward, I approved the change with the variant. This change undoes
the variant, and updates all the tests to just use the array form.
The CMake flag has been on by default for a month without any issues.
This makes the feature support in LLVM unconditional (but does not
enable the feature by default).
LLVM currently stores heapallocsite information in CodeView debuginfo,
but not in DWARF debuginfo. Plumb it into DWARF as an LLVM-specific
extension.
heapallocsite debug information is useful when it is combined with
allocator instrumentation that stores caller addresses; I've used a
previous version of this patch for:
- analyzing memory usage by object type
- analyzing the distributions of values of class members
Other possible uses might be:
- attributing memory access profiles (for example, on Intel CPUs, from
PEBS records with Linear Data Address) to object types or specific
object members
- adding type information to crash/ASAN reports
On X86-64, LLVM currently generates the same DWARF debug info for `call
rax` and `call [rax]`; in both cases, the generated DWARF claims that
the call goes to address RAX. This bug occurs because the X86 machine
instructions CALL64r and CALL64m both receive register operands, but
those register operands have different semantics.
To fix it, change DwarfDebug::constructCallSiteEntryDIEs() to validate
the callee operand's semantics (`OperandType`) and make sure it is not
semantically describing a memory location.
This fix will result in less DW_TAG_call_site and DW_AT_call_target
entries being generated.
There is an existing test in dwarf-callsite-related-attrs.ll that
asserts the broken behavior; remove the broken check, and instead add a
new test dwarf-callsite-related-attrs-indirect.ll that checks behavior
for indirect calls.
The existing test xray-custom-log.ll is validating something even more
broken: It checks the debug info generated by a PATCHABLE_EVENT_CALL.
`TII->getCalleeOperand()` assumes that the first argument of a call
instruction is always the destination, but the first argument of
PATCHABLE_EVENT_CALL is instead the event structure; and so we were
emitting debug info claiming the callee was stored in a register that
actually contains some kind of xray event descriptor, and the test
validates that this happens.
I am breaking and deleting this test.
I guess the intent there might have been to validate that we emit
debuginfo referencing the target of the direct call that LLVM emits
(which we don't do)? But I'm not sure.
LoongArch #77728 did better than RISCV.
If the label difference can be resolved (there is no linker-relaxable
fragment in between), we should use the default handling, which utilizes
a shorter special opcode.
This patch removes all uses of %T from within LLVM tests. %T has been
deprecated for about seven years and use is not advised given it is not
unique per test and can thus lead to races. The goal of this is to
eventually remove support for %T from lit.
fbf6271c7da20356d7b34583b3711b4126ca1dbb introduced an assertion failure as
setDebugValueUndef was called on DBG_LABELs, which isn't allowed and doesn't
make sense. Fix by skipping the call for DBG_LABELs and hoisting, in line with
the original behaviour.
Reapply #140091.
branch-folder hoists common instructions from TBB and FBB into their
pred. Without this patch it achieves this by splicing the instructions from TBB
and deleting the common ones in FBB. That moves the debug locations and debug
instructions from TBB into the pred without modification, which is not
ideal. Debug locations are handled in #140063.
This patch handles debug instructions - in the simplest way possible, which is
to just kill (undef) them. We kill and hoist the ones in FBB as well as TBB
because otherwise the fact there's an assignment on the code path is deleted
(which might lead to a prior location extending further than it should).
There's possibly something we could do to preserve some variable locations in
some cases, but this is the easiest not-incorrect thing to do.
Note I had to replace the constant DBG_VALUEs to use registers in the test- it
turns out setDebugValueUndef doesn't undef constant DBG_VALUEs... which feels
wrong to me, but isn't something I want to touch right now.
---
Fix end-iterator-dereference and add test.
MCObjectFileInfo::initMachOMCObjectFileInfo creates DWARF sections with
a temporary label as the `Begin` symbol, different from other object
file formats' section symbol.
#150574 caused a regression that removed the label for MCAsmStreamer,
which was caught by no test but a ystem-darwin/target-aarch64 specific diagnostics-dsym.test
Reapply #140091.
branch-folder hoists common instructions from TBB and FBB into their
pred. Without this patch it achieves this by splicing the instructions from TBB
and deleting the common ones in FBB. That moves the debug locations and debug
instructions from TBB into the pred without modification, which is not
ideal. Debug locations are handled in #140063.
This patch handles debug instructions - in the simplest way possible, which is
to just kill (undef) them. We kill and hoist the ones in FBB as well as TBB
because otherwise the fact there's an assignment on the code path is deleted
(which might lead to a prior location extending further than it should).
There's possibly something we could do to preserve some variable locations in
some cases, but this is the easiest not-incorrect thing to do.
Note I had to replace the constant DBG_VALUEs to use registers in the test- it
turns out setDebugValueUndef doesn't undef constant DBG_VALUEs... which feels
wrong to me, but isn't something I want to touch right now.
Finish making LLVM's implementation of `DW_LNCT_LLVM_source` conform to
the final accepted version of `DW_LNCT_source` from
https://dwarfstd.org/issues/180201.1.html
This is effectively a continuation of a few commits which have moved in
this direction already, including:
* c9cb4fc761cd7 [DebugInfo] Store optional DIFile::Source as pointer
* 87e22bdd2bd6d Allow for mixing source/no-source DIFiles in one CU
This patch:
* Teaches LLParser that there is a distinction between an empty and an
absent "source:" field on DIFile.
* Makes printing the "source:" field in AsmWriter conditional on it
being present, instead of being conditional on it being non-empty.
* Teaches MC to map an empty-but-present source field to "\n" (which is
ambiguous, making the source strings "" and "\n" indistinguishable, but
that's what the DWARF issue specifies).
Add a test for round-tripping an empty source field through
assembler/bitcode.
Extend the test for the actual DWARF generation so it covers all of the
cases (absent, present-but-empty,
present-and-ambiguously-single-newline, present).
branch-folder hoists common instructions from TBB and FBB into their
pred. Without this patch it achieves this by splicing the instructions from TBB
and deleting the common ones in FBB. That moves the debug locations and debug
instructions from TBB into the pred without modification, which is not
ideal. Debug locations are handled in pull request 140063.
This patch handles debug instructions - in the simplest way possible, which is
to just kill (undef) them. We kill and hoist the ones in FBB as well as TBB
because otherwise the fact there's an assignment on the code path is deleted
(which might lead to a prior location extending further than it should).
We might be able to do something smarter to preserve some variable locations in
some cases, but this is the easiest not-incorrect thing to do.
The verifier check was in the wrong place, meaning it wasn't actually
checking many instructions.
Fixing that causes a test failure (coro-dwarf-key-instrs.cpp) because
coros turn off the feature but still annotate instructions with the
metadata (which is a supported situation, but the verifier doesn't like
it, and it's hard to teach the verifier to like it).
Fix that by avoiding emitting any key instruction metadata if the
DISubprogram has opted out of key instructions.
A hardware loop instruction combines a subtract, compare with zero, and
branch. We currently account for the compare and branch being combined
into one in Cost::RateFormula, as part of more general handling for
compare-branch-zero, but don't account for the subtract, leading to
suboptimal decisions in some cases.
Fix this in Cost::RateRegister by noticing when we have such a subtract
and discounting the AddRecCost in such a case.
There isn't any way to encode a variable in an SVE register, and there
isn't any way to encode a scalable offset, and as far as I know that's
unlikely to change in the near future. So suppress any debug info which
would require those encodings.
This isn't ideal, but we need to ship something which doesn't crash.
Alternatively, for Z registers, we could emit debug info assuming the
vector length is 128 bits, but that seems like it would lead to
unintuitive results.
The change to AArch64FrameLowering is needed to avoid a crash. But we
can't actually test that the returned offset is correct: LiveDebugValues
performs the query, then discards the result.
This patch adds asserts to `getStackSizeSVE()` and
`getSVECalleeSavedStackSize()` to check they are only called after the
SVE stack size has been determined.
This patch then fixes issues in three places:
* `AArch64FrameLowering::homogeneousPrologEpilog()`
- This function is called before callee saves or SVE stack sizes have
been determined
- This check has been updated to use `isLikelyToHaveSVEStack()`
- `isLikelyToHaveSVEStack()` conservatively returns if a function is
likely to have an SVE stack
* `produceCompactUnwindFrame()`
- This function checked the SVE CS stack size before callee-saves had
been determined
- This has been replaced with a more conservative `isSVECC()` check
* `AArch64FrameLowering::resolveFrameOffsetReference()`
- This was hit by some post-PEI MIR tests
- This case was fixed by adding "stackSizeSVE" to the YAML for
`AArch64FunctionInfo`
PR #141106 changed the debug metadata to allow dynamic bit offsets and
sizes. In that patch, I forgot to update AsmWriter to handle this case.
This patch corrects the oversight.
This PR provides more information to debuggers and analysis tools on
Windows. It adds `S_LABEL32` symbols for each target BB of each jump
table. This allows debuggers to insert symbolic labels when
disassembling code. `S_LABEL32` symbol records indicate that a location
is definitely code, and can optionally associate a string label with the
code location. BBs generated for jump tables may or may not have string
labels, so it is acceptable for the "name" field within `S_LABEL32`
symbols to be an empty string.
More importantly, this PR allows Windows analysis tools, such as those
that generate hot-patches for the Windows kernel, to use these labels to
distinguish code basic blocks from data blocks. Microsoft's analysis
tools (similar to Bolt) rely on being able to identify all code blocks,
so that the tools can traverse all instructions and verify that
important requirements for hot-patching are met.
This PR has no effect on code generation. It only affects the CodeView
symbols that are emitted into OBJ files, which the linker then
repackages into PDB files.
Serialise key-instruction fields of DILocations and DISubprograms into
and outof bitcode, add tests. debug-info bitcode sizes grow, but it
balances out given an earlier size optimisation in 51f4e2c.
Co-authored-by: Orlando Cazalet-Hyams <orlando.hyams@sony.com>
This change removes unnecessary tune args to the AArch64 backend. The
AArch64 backend automatically handles `tune-cpu` and adds the necessar
y features based on the models from TableGen.
It follows this fix: https://github.com/llvm/llvm-project/pull/146260
where updating a subtarget feature didn't fail the frontend test because
both the toolchain and the test suffered from a coordinated error.
RFC on discourse:
https://discourse.llvm.org/t/rfc-debug-info-for-coroutine-suspension-locations-take-2/86606
With this commit, we add `DILabel` debug infos to the resume points of a
coroutine. Those labels can be used by debugging scripts to figure out
the exact line and column at which a coroutine was suspended by looking
up current `__coro_index` value inside the coroutines frame, and then
searching for the corresponding label inside the coroutine's resume
function.
The DWARF information generated for such a label looks like:
```
0x00000f71: DW_TAG_label
DW_AT_name ("__coro_resume_1")
DW_AT_decl_file ("generator-example.cpp")
DW_AT_decl_line (5)
DW_AT_decl_column (3)
DW_AT_artificial (true)
DW_AT_LLVM_coro_suspend_idx (0x01)
DW_AT_low_pc (0x00000000000019be)
```
The labels can be mapped to their corresponding `__coro_idx` values
either via their naming convention `__coro_resume_<N>` or using the new
`DW_AT_LLVM_coro_suspend_idx` attribute. In gdb, those line numebrs can
be looked up using `info line -function my_coroutine -label
__coro_resume_1`. LLDB unfortunately does not understand DW_TAG_label
debug information, yet.
Given this is an artificial compiler-generated label, I did apply the
DW_AT_artificial tag to it. The DWARFv5 standard only allows that tag on
type and variable definitions, but this is a natural extension and was
also blessed in the RFC on discourse.
Also, this commit adds `DW_AT_decl_column` to labels, not only for
coroutines but also for normal C and C++ labels. While not strictly
necessary, I am doing so now because it would be harder to do so later
without breaking the binary LLVM-IR format
Drive-by fixes: While reading the existing test cases to understand how
to write my own test case, I did a couple of small typo fixes and
comment improvements
Patch 3/4 adding bitcode support, though the final patch doesn't depend on this
one.
Prior to this patch, a Key Instructions function inlined into a
Not-Key-Instructions function fell back to Not-Key-Instructions handling.
In order to fully support inlining mixed modes we need to run
`computeKeyInstructions` (in case there's a Key Instructions scope) and
`findForceIsStmtInstrs` (in case there's a Not-Key-Instructions scope) on all
functions. This has a slight performance cost for all configurations - see PR
for details.
Patch 2/4 adding bitcode support.
A non-key-instructions function inlined into a key-instructions function uses
non-key-instructions is_stmt placement (without `findForceIsStmtInstrs`).
A key-instructions function inlined into a non-key-instructions function
currently results in falling back to non-key-instructions for the inlined scope
too.
Both of these concessions (not using `findForceIsStmtInstrs` in the 1st case,
and not using Key Instructions for the inlined scope in the 2nd) are for
performance reasons; to do the right thing we'd need to run both
`findForceIsStmtInstrs` and `computeKeyInstructions` - in case that's
controversial I've got a separate PR for that: PR 144103.
Patch 1/4 adding bitcode support.
Store whether or not a function is using Key Instructions in its DISubprogram so
that we don't need to rely on the -mllvm flag -dwarf-use-key-instructions to
determine whether or not to interpret Key Instructions metadata to decide
is_stmt placement at DWARF emission time. This makes bitcode support simple and
enables well defined mixing of non-key-instructions and key-instructions
functions in an LTO context.
This patch adds the bit (using DISubprogram::SubclassData1).
PR 144104 and 144103 use it during DWARF emission.
PR 44102 adds bitcode
support.
See pull request for overview of alternative attempts.
The insertion point of COPY isn't always optimal and could eventually
lead to a worse block layout, see the regression test in the first
commit.
This change affects many architectures but the amount of total
instructions in the test cases seems too be slightly lower.
Remove unneeded details like "<" and ">". Reduce indentation.
Omit `this` address to simplify output comparison.
Add a -debug-only=mc-dump test.
While here, add fixup printing for MCRelaxableFragment.
This changes the final stage of InstrRef, i.e. the TransferTracker
(which combines the values locations with the variable values), so that
it treats a DEBUG_VALUE of an EntryValue just like a DEBUG_VALUE of a
constant: a location that is never clobbered and can be propagated to
subsequent BBs as long as no other DEBUG_VALUE intrinsics updated the
variable.
We add two tests here:
1. `entry_value_clobbered_stack_copy` that saves a register on the
stack, uses this register as an entry value DBG_VALUE location, and then
clobbers it. Prior to this patch, this test would crash because we would
try to describe a new location for the variable in terms of what was
saved on the stack, and use an invalid expression to do so. This is not
needed as an EntryValue can never be clobbered.
2. `entry_value_gets_propagated`, that tests that an EntryValue
DBG_VALUE is propagated in a diamond-shaped CFG.
This patch is trying to reland
https://github.com/llvm/llvm-project/pull/77938 but also fixes the bug
with InstrRef based LiveDebugValues, where entry values were not being
propagated in a diamond-shaped CFG.
In Ada, a record type can have a non-constant size, and a field can
appear at a non-constant bit offset in a record.
To support this, this patch changes DIType to record the size and offset
using metadata, rather than plain integers. In addition to a constant
offset, both DIVariable and DIExpression are now supported here.
One thing of note in this patch is the choice of how exactly to
represent a non-constant bit offset, with the difficulty being that
DWARF 5 does not support this. DWARF 3 did have a way to support a
non-constant byte offset, combined with a constant bit offset within the
byte, but this was deprecated in DWARF 4 and removed from DWARF 5.
This patch takes a simple approach: a DWARF extension allowing the use
of an expression with DW_AT_data_bit_offset. There is a corresponding
DWARF issue, see https://dwarfstd.org/issues/250501.1.html. The main
reason for this approach is that it keeps API simplicity: just a single
value is needed, rather than having separate data describing the byte
offset and the bit within the byte.
Shaders compiled with DXC/LLPC generate these relocations, and even if
that changes in the future we want to handle existing binaries. The
friction to support this and the maintenance cost long term both seem
incredibly low, considering other targets like ARM support both REL/RELA
static relocations behind the same interface.
We started attaching `DW_AT_object_pointer`s on method declarations in
https://github.com/llvm/llvm-project/pull/122742. However, that caused
the `.debug_info` section size to increase significantly (by around ~10%
on some projects). This was mainly due to the large number of new
`DW_FORM_ref4` values. This patch tries to address that regression by
changing the `DW_FORM_ref4` to a `DW_FORM_implicit_const` for
declarations. The value of `DW_FORM_implicit_const` will be the *index*
of the object parameter in the list of formal parameters of the
subprogram (i.e., if the first `DW_TAG_formal_parameter` is the object
pointer, the `DW_FORM_implicit_const` would be `0`). The DWARFv5 spec
only mentions the use of the `reference` attribute class to for
`DW_AT_object_pointer`. So using a `DW_FORM_impilicit_const` would be an
extension to (and not something mandated/specified by) the standard.
Though it'd make sense to extend the wording in the spec to allow for
this optimization.
That way we don't pay for the 4 byte references on every attribute
occurrence. In a local build of clang this barely affected the
`.debug_info` section size (but did increase `.debug_abbrev` by up to
10%, which doesn't impact the total debug-info size much however).
We guarded this on LLDB tuning (since using `DW_FORM_implicit_const` for
this purpose may surprise consumers) and DWARFv5 (since that's where
`DW_FORM_implicit_const` was first standardized).
MSVC always emits minimal CodeView metadata with compiler information,
even when debug info is otherwise disabled. Other tools may rely on this
metadata being present. For example, linkers use it to determine whether
hotpatching is enabled for the object file.
This patch optimizes the Windows security cookie check mechanism by
moving the comparison inline and only calling __security_check_cookie
when the check fails. This reduces the overhead of making a DLL call
for every function return.
Previously, we implemented this optimization through a machine pass
(X86WinFixupBufferSecurityCheckPass) in PR #95904 submitted by
@mahesh-attarde. We have reverted that pass in favor of this new
approach. Also we have abandoned the AArch64 specific implementation
of same pass in PR #121938 in favor of this more general solution.
The old machine instruction pass approach:
- Scanned the generated code to find __security_check_cookie calls
- Modified these calls by splitting basic blocks
- Added comparison logic and conditional branching
- Required complex block management and live register computation
The new approach:
- Implements the same optimization during instruction selection
- Directly emits the comparison and conditional branching
- No need for post-processing or basic block manipulation
- Disables optimization at -Oz.
Thanks @tamaspetz, @efriedma-quic and @arsenm for their help.
Introduce MCAsmInfo::UsesSetToEquateSymbol to control the preferred
syntax for symbol equating. We now favor the more readable and common
`symbol = expression` syntax over `.set`. This aligns with pre- https://reviews.llvm.org/D44256 behavior.
On Apple platforms, this resolves a clang -S vs -c behavior difference (resolves#104623).
For targets whose = support is unconfirmed, UsesSetToEquateSymbol is set to false.
This also minimizes test updates.
Pull Request: https://github.com/llvm/llvm-project/pull/142289
By chance, two things have prevented the autoupgrade path being
exercised much so far:
* LLParser setting the debug-info mode to "old" on seeing intrinsics,
* The test in AutoUpgrade.cpp wanting to upgrade into a "new" debug-info
block.
In practice, this appears to mean this code path hasn't seen the various
invalid inputs that can come its way. This commit does a number of
things:
* Tolerates the various illegal inputs that can be written with
debug-intrinsics, and that must be tolerated until the Verifier runs,
* Printing illegal/null DbgRecord fields must succeed,
* Verifier errors need to localise the function/block where the error
is,
* Tests that now see debug records will print debug-record errors,
Plus a few new tests for other intrinsic-to-debug-record failures modes
I found. There are also two edge cases:
* Some of the unit tests switch back and forth between intrinsic and
record modes at will; I've deleted coverage and some assertions to
tolerate this as intrinsic support is now Gone (TM),
* In sroa-extract-bits.ll, the order of debug records flips. This is
because the autoupgrader upgrades in the opposite order to the basic
block conversion routines... which doesn't change the record order, but
_does_ change the use list order in Metadata! This should (TM) have no
consequence to the correctness of LLVM, but will change the order of
various records and the order of DWARF record output too.
I tried to reduce this patch to a smaller collection of changes, but
they're all intertwined, sorry.
When creating a static member DIE, we place it in a potentially
pre-existing context DIE, and that DIE might be located in a different
CU if we're in an LTO context. When we then add the source-file-ID to
the static member DIE, use the correct Unit to do so -- the one that
owns the context DIE. Otherwise we might assign a file-ID from one CU to
another, and there isn't a guarantee that they'll be the same file, or
even exist.
Fixes#109227
(I'd normally remove my home directory from these tests, but in this
circumstances the same-file-but-with-a-different-name nature of the
DIFile is part of the test).
This patch addresses an issue where an anonymous DWARF line table symbol
could be inadvertently defined multiple times, leading to an "symbol ''
is already defined" error during assembly or object file emission. This
issue happens for empty functions when
`-emit-func-debug-line-table-offsets` is enabled.
The root cause is the creation of the "end sequence" entry for a DWARF
line table. This entry was sometimes created by copying the last
existing line table entry. If this last entry was a special one (created
for the purpose of marking the position in the line table stream and
having an anonymous symbol attached), the copied end-sequence entry
would also incorrectly reference this same anonymous symbol.
Consequently, when the line table was finally emitted, the DWARF
emission logic would attempt to emit a label for this anonymous symbol
twice, triggering the redefinition error.
The fix ensures that when an end-sequence line table entry is created,
it does not inherit any special stream label from the entry it might
have been based on, thereby preventing the duplicate label emission.
Over in 6a45fce, this flag (experimental-debuginfo-iterators) was
switched to do nothing, to flush out anything that depended on the
debug-intrinsics way of doing things. It's been a month and nothing's
super-broken, so we'll start to rip things out.
This commit deletes MergeFunc's debuginfo-iterators test: in d2942a86d7
it's documented that that test is specifically because of differences
between intrinsic/non-intrinsic data structures, and we're deleting the
possibility of that difference.
These classes are redundant, as the untyped "Int" classes can be used
for all float operations. This change is intended to be as minimal as
possible and leaves the many potential simplifications and refactors
this exposes as future work.
