[lldb] Skip TestVariableAnnotationsDisassembler.py on unsupported architectures #155938
Conversation
This file contains hidden or 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
…DWARFExpressionList This introduces a new API for retrieving DWARF expression metadata associated with variable location entries at a given PC address. It provides the base, end, and expression pointer for downstream consumers such as disassembler annotations. Intended for use in richer instruction annotations in Instruction::Dump().
Co-authored-by: Jonas Devlieghere <[email protected]>
Updated comment for GetExpressionEntryAtAddress to directly refer to struct DWARFExpressionEntry Co-authored-by: Jonas Devlieghere <[email protected]>
updating code style for function GetExpressionEntryAtAddress Co-authored-by: Jonas Devlieghere <[email protected]>
Replace raw base/end with `AddressRange` in `DWARFExpressionEntry` and cleans up helper comments to follow Doxygen convention. Using `AddressRange` makes the intent clearer, avoids duplication of basic `AddressRange` logic usage
Converts `GetExpressionEntryAtAddress` to return `llvm::Expected<DWARFExpressionEntry>` using the updated `DWARFExpressionEntry`. Updates the implementation to compute a single `AddressRange file_range` for each DWARF location interval.
Updated commenting style for struct DWARFExpressionEntry
Updated function `llvm::Expected<DWARFExpressionList::DWARFExpressionEntry> DWARFExpressionList::GetExpressionEntryAtAddress` to use `FindEntryThatContains` instead of `FindEntryIndexThatContains`
Co-authored-by: Adrian Prantl <[email protected]>
This patch adds explicit checks:
- ensure `load_addr >= func_load_addr` to avoid underflow,
- compute and verify a temporary delta variable, then verify `delta + m_func_file_addr` does
not exceed `addr_t` max to avoid overflow.
…bly output. Right now just checks if DW annotations show up for a basic program and that a variable location is annotated (i.e 'a = DW_OP_reg...').
- Fixed an issue where variable location annotations were not shown if the current instruction address did not exactly match the DWARF base address. Now, annotations are shown as long as the PC is within the valid range. - Improved alignment of annotation comments in Instruction::Dump(). While `FillLastLineToColumn` can sometimes overcompensate due to internal formatting or byte-width mismatches, the overall alignment is now significantly more consistent and readable.
Previously, when a DWARF expression contained any decoding error, the entire variable location annotation was printed with the error, e.g. `c = DW_OP_addr 0x0, <decoding error> 00 00 00`. This was misleading and cluttered the disassembly view. This patch improves the formatting by stripping out the `<decoding error ...>` fragments while preserving the valid portions of the expression, so that partial information like `c = DW_OP_addr 0x0` can still be shown. This allows the rich disassembler to give more meaningful variable annotations, especially in optimized (-O1/-O2) builds where partial DWARF corruption or unsupported expressions may occur.
Handled edge case where the entire DWARF expression is a `<decoding error>`, ensuring no misleading or empty annotations are printed for such variables.
This patch adds API tests to verify that DWARF variable location annotations are correctly displayed in the disassembly output. The tests cover: - Local variables in loops and functions - Multiple stack variables - Control flow edge cases - Different optimization levels (-O0, -O1, -O2) - Ensuring decoding errors are excluded from output
…try API This rebases the `add-disassembler-annotations` work onto the latest `add-dwarfexprentry-api` branch so that the instruction annotation patches sit cleanly atop the new DWARFExpressionEntry struct and helper API. All conflicts have been resolved and the annotation code now integrates with the updated std::optional<AddressRange>-based GetExpressionEntryAtAddress signature.
…w `DWARFExpression::DumpLocationWithOptions` for simplified expression printing This patch introduces a PrintRegisterOnly flag to the DIDumpOptions struct, enabling concise rendering of DWARF expressions for disassembler annotations. Key changes: - Added DumpLocationWithOptions to DWARFExpression for flexible dumping with DIDumpOptions. - Updated DWARFExpression::print and Operation::print to respect PrintRegisterOnly, rendering registers like RDI without DW_OP_ or llvm: prefixes. - Suppressed <decoding error> output when PrintRegisterOnly is true to avoid clutter during register-only disassembly output. These changes are motivated by LLDB’s rich disassembler feature, where annotations should match user-facing register names without DWARF-level detail. Test impact: Some rich-disassembler tests that relied on DW_OP_ for validation were deprecated. Updated tests aligned with the new formatting will be added next.
…n Instruction::Dump
…tate_variable` lambda function
…IDumpOptions instead of having to introduce new function
Resolve conflicts in DWARFExpression files after upstream refactor
This change refactors how the `--rich` flag is handled, based on code review feedback.
- Removed the `enable_rich_annotations` boolean from the API signatures of:
- Disassembler::Disassemble(...)
- Disassembler::PrintInstructions(...)
- StackFrame::Disassemble(...)
- Added a new Disassembler::Option enum value: eOptionRichAnnotations.
- The `--rich` CLI flag now sets the new option bit in CommandObjectDisassemble::DoExecute:
options |= Disassembler::eOptionRichAnnotations;
- Disassembler::PrintInstructions checks the bit to determine whether to enable rich annotations:
const bool enable_rich = (options & eOptionRichAnnotations) != 0;
The SB API remains unchanged and defaults to non-rich output.
Tested via the existing test using `disassemble --rich -f`.
Address code review feedback suggesting the use of LLVM's DenseMap family over std::unordered_map for consistency and potential performance benefits within LLDB. Replaced: std::unordered_map<lldb::user_id_t, VarState> with: llvm::SmallDenseMap<lldb::user_id_t, VarState, 8> The small buffer size of 8 is a heuristic for typical numbers of live variables in scope, reducing allocations for common cases.
Co-authored-by: Adrian Prantl <[email protected]>
Rework the variable-location annotation logic to operate purely on
file addresses and DWARF data so it doesn’t require a Process/StackFrame.
This enables annotations when disassembling static objects (or in crash/core
scenarios) and sets the stage for portable tests built from yaml2obj.
Key changes:
- Resolve context at the instruction’s *file* address and use
eSymbolContextFunction | eSymbolContextBlock. Use sc.block (innermost)
and Block::AppendVariables(can_create=true, get_parent_variables=true,
stop_if_block_is_inlined_function=false, filter, &list) to collect
in-scope, non-artificial variables (via a filter callback).
-- Pretty-print register names through an ABI derived from the Target only:
ABI::FindPlugin(ProcessSP(), ArchSpec). Handle absence gracefully; fall
back to DWARF reg ops.
- Keep a small live map keyed by Variable user_id_t and emit only
transitions:
- “name = REG” on start/change
- “name = <undef>” when a location disappears
This patch extends `TestRichDisassembler.py` (in lldb/test/API/functionalities/disassembler-variables) with a suite of new object-level test cases, complementing the existing `d_original_example.c` test. Each case exercises different calling conventions and register lifetimes, ensuring disassembler annotations are validated in a portable way. - Keep original style test for `d_original_example.c` (full exe, stop in main).
Refactor `TestRichDisassembler.py` to use pre-generated `.s` files instead of compiling C sources directly. This ensures stable, portable test inputs that don’t depend on compiler optimizations or register allocation heuristics. - Added `.s` assembler files alongside the original `.c` sources for reference. - Updated test harness with `_compile_or_assemble_object()` helper: * Detects `.s` inputs and assembles them with clang (`-x assembler`). * Falls back to compiling `.c` if needed. - Adjusted regex assertions to tolerate both DWARF register numbers and architecture-specific register names. - Fixed placeholder checks in `loop_reg_rotate` and relaxed return register checks for `live_across_call`. These changes make the disassembler-variables tests deterministic and portable while still keeping the original `.c` sources around for documentation and future regeneration of the `.s`.
…terOnly Previously, when `DIDumpOptions::PrintRegisterOnly` was enabled, the disassembler annotations only displayed registers. This omitted useful cases where variables are assigned to constant values (e.g., `i = 0`). This change updates the DWARF expression printer to still emit constants when `PrintRegisterOnly` is active, while continuing to suppress other DWARF opcodes. As a result, users now see clearer annotations for constants alongside registers.
Co-authored-by: Adrian Prantl <[email protected]>
…tionsDisassembler.
Co-authored-by: Adrian Prantl <[email protected]>
Co-authored-by: Adrian Prantl <[email protected]>
The test TestVariableAnnotationsDisassembler.py relies on an .s input file generated for x86_64 with assembly directives that are not portable (e.g., DWARF sections and comments using #). This causes build failures on non-x86 targets such as AArch64.
tstellar
requested changes
Aug 28, 2025
Collaborator
tstellar
left a comment
tstellar
left a comment
There was a problem hiding this comment.
This has a lot of unrelated changes, can you remove them.
Contributor
Author
Sorry I will close this PR, can create a new one from a fresh branch so it just shows the necessary change. |
Contributor
Author
|
New PR created with just lldb tester change for TestVariableAnnotationsDisassembler: #155942 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The test lldb-api::TestVariableAnnotationsDisassembler.py was failing on the lldb-remote-linux-ubuntu and lldb-remote-linux-win builders due to assembler incompatibilities in d_original_example.s. These failures are not related to the disassembler changes themselves but to the test setup.
This patch updates the test to be skipped when running on unsupported architectures to avoid failures. The test will still run and validate correctly where the assembler input is supported.