Removed the duplicated function body in the Cortex-M4 AC6 port - #589
Merged
fdesbiens merged 1 commit intoAug 9, 2026
Merged
Conversation
_tx_thread_system_return_inline() in the Cortex-M4 AC6 tx_port.h was followed by a second, orphaned copy of its own body. The copy had no function header, so it declared interrupt_save at file scope and then placed statements there, which does not compile. It is also the older version of the body, without the dsb and isb barriers, so it was left behind rather than intended: the barriers were added by commit 33efad3 and the previous text was not removed. Delete the orphaned copy. What remains is the same body every sibling port carries: after this change ports/cortex_m4/ac6/inc/tx_port.h differs from ports/cortex_m4/iar/inc/tx_port.h and ports/cortex_m7/ac6/inc/tx_port.h only in the port name in the banner and the version string, as it should. Verified by compiling the function in isolation, which fails on the file scope statements before the change and is clean afterwards, and by a structural scan of all 208 tx_port.h files in the repository confirming this was the only occurrence. Fixes eclipse-threadx#569 Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
This was referenced Aug 9, 2026
akifejaz
pushed a commit
to akifejaz/threadx
that referenced
this pull request
Aug 17, 2026
…se-threadx#589) _tx_thread_system_return_inline() in the Cortex-M4 AC6 tx_port.h was followed by a second, orphaned copy of its own body. The copy had no function header, so it declared interrupt_save at file scope and then placed statements there, which does not compile. It is also the older version of the body, without the dsb and isb barriers, so it was left behind rather than intended: the barriers were added by commit 33efad3 and the previous text was not removed. Delete the orphaned copy. What remains is the same body every sibling port carries: after this change ports/cortex_m4/ac6/inc/tx_port.h differs from ports/cortex_m4/iar/inc/tx_port.h and ports/cortex_m7/ac6/inc/tx_port.h only in the port name in the banner and the version string, as it should. Verified by compiling the function in isolation, which fails on the file scope statements before the change and is clean afterwards, and by a structural scan of all 208 tx_port.h files in the repository confirming this was the only occurrence. Fixes eclipse-threadx#569 Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
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
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.
Fixes #569, reported by @AKevin99.
The defect
_tx_thread_system_return_inline()inports/cortex_m4/ac6/inc/tx_port.hwas followed by a second, orphaned copy of its own body:The copy has no function header, so it declares
interrupt_saveat file scope and then places statements there. That does not compile:It sits inside
#if defined(__GNUC__) || defined(__ICCARM__), which ARM Compiler 6 satisfies, and outsideTX_DISABLE_INLINE, so it is reached by a default AC6 build of this port.The orphan is also the older text: it lacks the
dsbandisbbarriers that the live copy above it has. Those barriers were added by 33efad3 (#523), and the previous version was not deleted, which matches the "small oversight when improving the code" in the report.The fix
Delete the orphaned copy. Nothing else changes.
What remains is exactly the body every sibling carries. After this change,
ports/cortex_m4/ac6/inc/tx_port.hdiffers fromports/cortex_m4/iar/inc/tx_port.hand fromports/cortex_m7/ac6/inc/tx_port.honly in the port name in the banner and the version string.Worth stating explicitly: the surviving body is correct as it stands and needs no barrier added. Diffing AC6 against the GNU sibling suggests otherwise at first glance, because the orphan lines up against a trailing
isbthat GNU has, but grouping all eighteen M3/M4/M7 headers by their actual function body shows AC6 belongs with IAR, which has no trailingisb. Adding one here would have been a real behavioural change smuggled in under a cleanup.Verification
tx_port.hfiles in the repository were scanned structurally, tracking brace depth and ignoring preprocessor lines, for a PendSV write occurring outside any function body. This file was the only hit, both before and after.Related divergence, not addressed here
Grouping the eighteen M3/M4/M7 headers by that function body turns up three distinct versions of code that
scripts/copy_armv7_m.shdescribes as coming from a single source:dsb+isbat entry, no trailingisbdsb+isbat entry, plus a trailingisbafter__restore_interrupt()ports_arch/ARMv7-M/threadx/inc/tx_port.hThe last row is the one to look at.
scripts/copy_armv7_m.shsays "There is only one tx_port.h file that covers three architectures: M3/M4/M7 and four tools: ac5/ac6/gnu/iar", and the file it copies from is in the no-barrier group. Running that script today would therefore revert #523 across every M3, M4 and M7 port that currently has the barriers.That is out of scope for a fix to #569. I will address that in one or more PRs, however.