fix(debug): disable .debug_names to fix GDB warning with multi-CU binaries#1675
fix(debug): disable .debug_names to fix GDB warning with multi-CU binaries#1675ghaith wants to merge 4 commits into
Conversation
…control The LLVM C API (LLVMDIBuilderCreateCompileUnit) does not expose the NameTableKind parameter, which controls .debug_names accelerator table emission. This adds a C++ wrapper that calls DIBuilder::createCompileUnit directly, allowing us to set NameTableKind::None (matching clang's default) or NameTableKind::Default for opt-in .debug_names emission. The Rust side exposes a safe `create_debug_info()` function that hides all raw pointer handling and transmutes behind a friendly inkwell-typed API.
…ility When compiling multiple ST files with DWARF 5 debug info, each compilation unit gets its own .debug_names table. The linker (lld) concatenates them, but GDB cannot parse concatenated tables and warns: "Section .debug_names length X does not match section length Y" This matches clang's default behavior of setting nameTableKind: None on DICompileUnit. A new --gpubnames CLI flag allows opting back into .debug_names emission when needed (e.g. for lldb-only workflows).
All debug test snapshots now include nameTableKind: None in the DICompileUnit metadata, reflecting the new default behavior.
Build Artifacts🐧 Linux
From workflow run 🪟 Windows
From workflow run |
There was a problem hiding this comment.
Pull request overview
This PR changes how DWARF compile units are created so .debug_names (DWARF v5 accelerator table) is disabled by default to avoid GDB warnings when linking multi-compilation-unit binaries, while adding a CLI opt-in to re-enable it.
Changes:
- Add a
plc_llvmC++/FFI wrapper aroundllvm::DIBuilder::createCompileUnitto controlDICompileUnit::DebugNameTableKind. - Thread a new
generate_pubnamesoption fromplc_driverCLI (--gpubnames) through codegen into debug info creation. - Update debug-info-related snapshot expectations to include
nameTableKind: None.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| compiler/plc_driver/src/cli.rs | Adds --gpubnames flag to opt into .debug_names emission. |
| compiler/plc_driver/src/lib.rs | Adds generate_pubnames to CompileOptions with default false. |
| compiler/plc_driver/src/pipelines.rs | Propagates CLI flag into CompileOptions and into CodeGen::new. |
| compiler/plc_llvm/src/cpp/llvm_wrapper.cpp | Implements C++ wrapper createCompileUnit selecting DebugNameTableKind::{None,Default}. |
| compiler/plc_llvm/src/lib.rs | Adds create_debug_info API that calls the C++ wrapper and returns inkwell debug builder/CU wrappers. |
| src/codegen.rs | Extends CodeGen::new to accept and pass through generate_pubnames. |
| src/codegen/debug.rs | Switches to plc_llvm::create_debug_info(...) and forwards generate_pubnames. |
| src/test_utils.rs | Updates test helper CodeGen::new call sites for the new parameter. |
| compiler/plc_driver/src/tests/snapshots/plc_driver__tests__multi_files__multiple_files_with_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| compiler/plc_driver/src/tests/snapshots/plc_driver__tests__multi_files__multiple_files_in_different_locations_with_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| tests/integration/snapshots/tests__integration__cfc__ir__actions_debug.snap | Snapshot update to expect nameTableKind: None. |
| tests/integration/snapshots/tests__integration__cfc__ir__conditional_return_debug.snap | Snapshot update to expect nameTableKind: None. |
| tests/integration/snapshots/tests__integration__cfc__ir__jump_debug.snap | Snapshot update to expect nameTableKind: None. |
| tests/integration/snapshots/tests__integration__cfc__ir__sink_source_debug.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/oop_tests/debug_tests.rs | Updates embedded IR expectations for nameTableKind: None. |
| src/codegen/tests/debug_tests.rs | Updates embedded IR expectations for nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__dwarf_version_override.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_alias_type.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_array_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_byteseq_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_float_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_int_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_nested_struct_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_pointer_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_string_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_struct_added_to_debug_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__aggregate_return_value_variable_in_function.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__array_size_correctly_set_in_dwarf_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__assignment_statement_have_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__case_conditions_location_marked.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__exit_statement_have_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__external_impl_is_not_added_as_external_subroutine.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__for_conditions_location_marked.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__function_calls_have_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__function_calls_in_expressions_have_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__if_conditions_location_marked.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__implementation_added_as_subroutine.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__nested_function_calls_get_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__non_callable_expressions_have_no_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__non_function_pous_have_struct_as_param.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__repeat_conditions_location_marked.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__return_statement_have_location.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__string_size_correctly_set_in_dwarf_info.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__var_and_vartemp_variables_in_pous_added_as_local.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__var_in_out_inout_in_function_added_as_params.snap | Snapshot update to expect nameTableKind: None. |
| src/codegen/tests/debug_tests/snapshots/rusty__codegen__tests__debug_tests__expression_debugging__while_conditions_location_marked.snap | Snapshot update to expect nameTableKind: None. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let (debug_info, compile_unit) = plc_llvm::create_debug_info( | ||
| module, | ||
| true, | ||
| inkwell::debug_info::DWARFSourceLanguage::C, //TODO: Own lang | ||
| filename, | ||
| root.to_str().unwrap_or_default(), | ||
| "RuSTy Structured text Compiler", | ||
| optimization.is_optimized(), | ||
| "", | ||
| 0, | ||
| "", | ||
| debug_level.into(), | ||
| 0, | ||
| false, | ||
| false, | ||
| "", | ||
| "", | ||
| generate_pubnames, | ||
| ); |
There was a problem hiding this comment.
New behavior is gated on generate_pubnames, but the updated snapshots only cover the default (false) path. Add a test (or a snapshot variant) that sets generate_pubnames = true / passes --gpubnames and asserts the compile unit uses the non-None nameTableKind (i.e., .debug_names is emitted) so this opt-in path can’t regress silently.
| assert!(size_of::<DebugInfoBuilder>() == size_of::<usize>() + size_of::<PhantomData<&()>>()); | ||
| assert!( | ||
| size_of::<DICompileUnit>() | ||
| == size_of::<DIFile>() + size_of::<usize>() + size_of::<PhantomData<&()>>() | ||
| ); |
There was a problem hiding this comment.
The compile-time layout verification block won’t compile as written: size_of isn’t in scope and the generic inkwell types require an explicit lifetime parameter (e.g., DebugInfoBuilder<'_>, DICompileUnit<'_>, DIFile<'_>). Use std::mem::size_of::<...>() with <'_> (or import size_of) so this crate builds.
| assert!(size_of::<DebugInfoBuilder>() == size_of::<usize>() + size_of::<PhantomData<&()>>()); | |
| assert!( | |
| size_of::<DICompileUnit>() | |
| == size_of::<DIFile>() + size_of::<usize>() + size_of::<PhantomData<&()>>() | |
| ); | |
| assert!( | |
| std::mem::size_of::<DebugInfoBuilder<'_>>() | |
| == std::mem::size_of::<usize>() + std::mem::size_of::<PhantomData<&()>>() | |
| ); | |
| assert!( | |
| std::mem::size_of::<DICompileUnit<'_>>() | |
| == std::mem::size_of::<DIFile<'_>>() | |
| + std::mem::size_of::<usize>() | |
| + std::mem::size_of::<PhantomData<&()>>() | |
| ); |
| let debug_info: DebugInfoBuilder<'ctx> = std::mem::transmute(builder_ref); | ||
| let compile_unit: DICompileUnit<'ctx> = | ||
| std::mem::transmute((file_ref, cu_ref, PhantomData::<&'ctx ()>)); | ||
|
|
There was a problem hiding this comment.
std::mem::transmute is being used to construct DebugInfoBuilder/DICompileUnit from raw pointers/tuples. This is only sound if those inkwell wrapper types have a guaranteed representation compatible with the transmuted values; size checks + debug asserts don’t make the layout guarantee in release builds. Prefer an API that constructs these wrappers from raw LLVM pointers (or add such a constructor in a small local wrapper layer) to avoid relying on inkwell’s private layout.
Summary
this parameter
Context
When compiling multiple ST files with DWARF 5 debug info, each compilation unit generates its own .debug_names accelerator table. lld
concatenates these into a single ELF section, but GDB cannot parse concatenated tables and warns:
warning: Section .debug_names length X does not match section length Y, ignoring .debug_names.Neither GCC nor clang emit .debug_names by default — clang explicitly sets
nameTableKind: Noneon itsDICompileUnit. The LLVM C API (LLVMDIBuilderCreateCompileUnit) does not expose theNameTableKindparameter, so we bypass it with a C++ wrapper that calls the C++DIBuilder::createCompileUnitdirectly.