feat!: make sure constructors don't get optimised out#1540
Conversation
18437fb to
750d3e5
Compare
38bae86 to
8b4e085
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1540 +/- ##
==========================================
- Coverage 94.95% 94.64% -0.31%
==========================================
Files 174 187 +13
Lines 56307 58487 +2180
==========================================
+ Hits 53464 55354 +1890
- Misses 2843 3133 +290 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4615f6b to
943e9fe
Compare
1d77ab9 to
6745bed
Compare
adds a new plc_llvm crate that wraps llvm c++ apis. The only api currently provided is to expose the init array flag. This ensures that the constructors created by the compiler are actually called in aarch64 architectures. filter out alignments so we are compatible with aarch64 filter debug align info add alignment tests
6745bed to
fb001ba
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to ensure constructors are not optimized out by using the ELF .init_array section. It removes built-in linker scripts, deprecates the --no-linker-script flag, and enables builds on aarch64 (arm64) architecture. To support cross-architecture compatibility, alignment values in test snapshots have been filtered out.
Changes:
- Modified LLVM global constructors to use priority 65535 instead of 0 for the
.init_arraysection - Updated all test snapshots to replace specific alignment values with
[filtered]placeholders - Added a new alignment_tests module to handle architecture-specific alignment testing
Reviewed changes
Copilot reviewed 295 out of 407 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/codegen/tests/snapshots/*.snap | Updated test snapshots to filter out alignment values for cross-architecture compatibility |
| src/codegen/tests.rs | Added new alignment_tests module |
Comments suppressed due to low confidence (3)
src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_array_added_to_debug_info.snap:1
- The removal of the
snapshot_kind: textmetadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---
src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_alias_type.snap:1
- The removal of the
snapshot_kind: textmetadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---
src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__dwarf_version_override.snap:1
- The removal of the
snapshot_kind: textmetadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...sts/snapshots/rusty__codegen__tests__debug_tests__global_var_struct_added_to_debug_info.snap
Show resolved
Hide resolved
...pshots/rusty__codegen__tests__debug_tests__global_var_nested_struct_added_to_debug_info.snap
Show resolved
Hide resolved
...s/plc_driver__tests__multi_files__multiple_files_in_different_locations_with_debug_info.snap
Show resolved
Hide resolved
...iver/src/tests/snapshots/plc_driver__tests__multi_files__multiple_files_with_debug_info.snap
Show resolved
Hide resolved
The tests had issues with debug info, the problem was that when generating debug info, we should have skipped extenrnal and init functions. Also removed old comments
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 295 out of 408 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add ubuntu-24.04-arm matrix entry (aarch64) and include it when creating and pushing the Docker manifest.
Adds an LLVM C++ Wrapper that provides access to mark the compiled application to use the elf .init_array constructors.
Removes the built in linker scripts and deprecates the --no-linker-script flag.
This is a breaking change as the init functions are no longer visible in the binary.
This PR also activates builds on aarch64 (arm64) and for that we needed to filter out alignmets from all the tests.