fix: debug info alignment #1468
Open
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
mhasel
force-pushed
the
debug-alignment
branch
2 times, most recently
from
187a5e7 to
e8d2520
Compare
While not setting an explicit alignment doesn't make much of a difference for x86-based target architectures, it is still recommended when targetting MIPS or ARM ISAs. See https://llvm.org/docs/Frontend/PerformanceTips.html#when-to-specify-alignment
mhasel
commented
Apr 17, 2025
mhasel
changed the title
…_debug_tests__expression_debugging__zero_sized_types_do_not_have_alignments.snap.new
There was a problem hiding this comment.
Copilot reviewed 23 out of 39 changed files in this pull request and generated no comments.
Files not reviewed (16)
- compiler/plc_driver/src/tests/snapshots/plc_driver__tests__multi_files__multiple_files_in_different_locations_with_debug_info.snap: Language not supported
- compiler/plc_driver/src/tests/snapshots/plc_driver__tests__multi_files__multiple_files_with_debug_info.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__aggregate_return_value_variable_in_function.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__array_size_correctly_set_in_dwarf_info.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__assignment_statement_have_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__case_conditions_location_marked.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__exit_statement_have_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__external_impl_is_not_added_as_external_subroutine.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__function_calls_have_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__function_calls_in_expressions_have_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__if_conditions_location_marked.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__implementation_added_as_subroutine.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__nested_function_calls_get_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__non_callable_expressions_have_no_location.snap: Language not supported
- src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__non_function_pous_have_struct_as_param.snap: Language not supported
Comments suppressed due to low confidence (3)
src/codegen/debug.rs:331
- [nitpick] Consider adding a comment to document why alignment adjustments are only applied when align_bits exceeds 8, clarifying the rationale for handling byte-aligned fields differently.
if align_bits > 8 {
src/codegen/debug.rs:387
- [nitpick] Consider renaming this variable (e.g., to 'debug_struct') to avoid confusion with the earlier LLVM struct type used in alignment calculations.
let struct_type = self.debug_info.create_struct_type(
src/codegen/debug.rs:614
- When get_associated_type returns None, alignment defaults to 0; consider handling this scenario explicitly to ensure consistent alignment behavior for debug variables.
let align_bits = types_index
a0c0310 to
02aff74
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 PR reintroduces explicit alignments in our debug-info. Additionally, for struct-types, the offsets are now manually calculated and then compared with the LLVM-offset, choosing the bigger value. This prevents overlap when reading values from memory during debugging, which would lead to corrupted values.
In particular, this issue arose for variables placed after 64-bit types. In the example below, the running offset didn't match the actual offset in memory. 304 + 8 != 308.
(gdb) ptype /o <struct>:After reading a bit further into the LLVM documentation/performance guide for frontend authors, I have decided to enable alignment for each variable-type except globals. While alignment can safely be omitted for x86 architectures with little-to-no performance drawbacks, it is still recommended for MIPS and ARM ISAs.
When to specify alignment
For global variables, however, it is recommended to let the target choose how it wants to align variables in memory. Specifying an alignment here would force the target to use the specified alignment and prevent targets and optimizers from over-aligning.
Global vars
While these points are generally not written in regards to debug-information, I think i doesn't hurt to match these specifications when generating DI aswell.