Conversation
A stray IAR installer download URL was left outside the comment delimiters
in the restore_special_regs section of the IAR TrustZone non-secure ports.
The assembler parses the bare URL as an operand of the preceding ldmia, so
any build of these ports aborts with:
Error: garbage following instruction -- 'ldmia r2!,{r0,r3,lr} http:'
The line is outside the configENABLE_PAC conditional block, so it breaks
every configuration of the port, not only PAC-enabled builds.
Commit 78e0cc7 ("ARMv8.1-M: Add task dedicated PAC key support", FreeRTOS#1195)
introduced the URL in the CM33/CM35P/CM55/CM85 and ARMv8M non-secure
ports. The CM52 (FreeRTOS#1334) and STAR-MC3 (FreeRTOS#1363) ports inherited it when
their portasm.s was created from an already-affected file.
The stray URL is removed rather than moved inside the comment, matching
the unaffected sibling port ARM_CM23/non_secure/portasm.s, which carries
the same comment text without any URL.
Verified with arm-none-eabi-gcc 13.3.1: all six files fail to assemble
before the change and assemble cleanly after it.
Relates to FreeRTOS#1480. The ARM_CM33 non-secure port is handled separately by
FreeRTOS#1485, so it is intentionally not touched here.
8611557 to
2b4dfaf
Compare
|
Hi @sekior11, would you mind including that last file with these changes? PR #1485 uses a different fix, but your fix is preferable. You can simply execute portable/ARMv8M/copy_files.py and then commit the results. It will be just that one file. Then maintainers can simply close #1485 without merging it. |
portable/ARMv8M/copy_files.py derives the non-secure IAR ports from portable/ARMv8M/non_secure/portable/IAR/ARM_CM33. The previous commit fixed that source directory but did not regenerate the ARM_CM33 copy, so portable/IAR/ARM_CM33/non_secure/portasm.s still carried the stray URL. mpu_wrappers_v2_asm.S and portmacro.h are already identical between source and destination, so re-running the script changes only this file. With this commit all seven files that carry the stray URL are fixed, which makes FreeRTOS#1485 (same file, same line) unnecessary. Correction to the previous commit message: the stray URL is in the #else branch of #if ( configENABLE_MPU == 1 ), so it is not every configuration of the port. It breaks every non-MPU build, independently of configENABLE_PAC; MPU builds never compile the line. Verified with arm-none-eabi-gcc 13.3.1 (GNU as, not IAR iccarm), using -march=armv8.1-m.main -mthumb -mfpu=fpv5-d16 and a minimal IAR-to-GNU directive translation: configENABLE_MPU=0, configENABLE_PAC=0 fails before, assembles after configENABLE_MPU=1 line is not compiled The PAC_KEY_* mrs/msr instructions are rejected by GNU as on this target, so the PAC-enabled non-MPU configuration could not be exercised here. Relates to FreeRTOS#1480.
|
|
@jefftenney Done — added You were right about the script. I checked what it actually does before committing, since I wanted to be sure it would not drag in unrelated churn: Re-verifying also turned up a mistake of mine that I have corrected. The original description said the stray URL "breaks every configuration of these ports". That is too strong: the line is outside the Thanks for the pointer — this makes #1485 unnecessary. |
3 participants
Problem
A stray IAR installer download URL sits outside the comment delimiters in the
restore_special_regssection of the IAR TrustZone non-secure ports:Because
/*appears after the URL, the assembler reads the bare URL as an operandof the preceding
ldmiaand aborts:The line sits outside the
#if ( configENABLE_PAC == 1 )block, which ends on thepreceding line, so it is not limited to PAC-enabled builds. It is, however, inside the
#elsebranch of#if ( configENABLE_MPU == 1 ): the breakage is on every non-MPUbuild, independently of
configENABLE_PAC, while MPU builds never compile the line.(Corrected — an earlier revision of this description said "breaks every configuration
of these ports". That was too strong; the guard stack for line 476 is
#else of #if ( configENABLE_MPU == 1 )and nothing else.)Scope
Issue #1480 reports this for
portable/IAR/ARM_CM33/non_secure/portasm.s.A tree-wide scan of
main(8be86d4) finds the stray URL in 7 files, all atline 476. This PR fixes all seven:
portable/ARMv8M/non_secure/portable/IAR/ARM_CM33/portasm.sportable/IAR/ARM_CM33/non_secure/portasm.sportable/IAR/ARM_CM35P/non_secure/portasm.sportable/IAR/ARM_CM52/non_secure/portasm.sportable/IAR/ARM_CM55/non_secure/portasm.sportable/IAR/ARM_CM85/non_secure/portasm.sportable/IAR/ARM_STAR_MC3/non_secure/portasm.sThe first revision covered only the last six, leaving
portable/IAR/ARM_CM33/non_secure/portasm.sto #1485. Per @jefftenney's reviewcomment that split no longer makes sense:
portable/ARMv8M/copy_files.pyderives thenon-secure IAR ports from
portable/ARMv8M/non_secure/portable/IAR/ARM_CM33— the wholedirectory for ARM_CM33 itself, file by file for the derivatives — so running the script
after the first commit produces the ARM_CM33 fix as well. It is included here and
#1485 becomes redundant.
Note there are two distinct CM33 assembly files in the tree, both affected, and they
are easy to confuse:
portable/IAR/ARM_CM33/non_secure/portasm.sportable/ARMv8M/non_secure/portable/IAR/ARM_CM33/portasm.sFix
The stray URL is deleted, and the comment keeps its original text and column alignment:
This matches the unaffected sibling port
portable/IAR/ARM_CM23/non_secure/portasm.s(line 444), which carries the same comment text without any URL. Optionally the URL
could be kept inside the comment instead, as #1485 does — either form assembles; I
removed it because it is a meaningless installer link.
Origin
Commit 78e0cc7 ("ARMv8.1-M: Add task dedicated PAC key support", #1195) introduced the
URL into CM33, CM35P, CM55, CM85 and the ARMv8M non-secure port. The CM52 (#1334) and
STAR-MC3 (#1363) ports were later created from an already-affected
portasm.sandinherited it.
Verification
Assembled with
arm-none-eabi-gcc 13.3.1 20240614(GNU Tools for STM32 13.3.rel1),-march=armv8.1-m.main -mthumb -mfpu=fpv5-d16. IAR's own directives(
PUBLIC/EXTERN/SECTION/THUMB/END) were translated 1:1 to GNUequivalents so that the only difference between the "before" and "after" inputs is the
one comment line, making the result attributable to it. The translated pair still
diffs in exactly that single line.
Baseline (known-good sibling, proves the harness accepts a healthy file):
All seven files in this PR, over
configENABLE_MPU/configENABLE_PAC:portable/IAR/ARM_CM33/non_secure/portasm.sportable/IAR/ARM_CM35P/non_secure/portasm.sportable/IAR/ARM_CM52/non_secure/portasm.sportable/IAR/ARM_CM55/non_secure/portasm.sportable/IAR/ARM_CM85/non_secure/portasm.sportable/IAR/ARM_STAR_MC3/non_secure/portasm.sportable/ARMv8M/non_secure/portable/IAR/ARM_CM33/portasm.sCaveats, stated plainly:
iccarm(no IAR licence available here). Both reject abare token after an instruction, and the reporter of [BUG] IAR ARMCM33 Non-Secure portasm.s stray URL outside comment delimiters causes "Invalid syntax" assembler error in portasm.s #1480 reproduced the failure on
IAR ARM 9.70.2, so the defect and the fix are cross-confirmed — but this is not an
IAR build.
PAC_KEY_*mrs/msroperands on this target, before it ever reaches line 476.Note on CI
.github/workflows/kernel-demos.ymlonly builds theCORTEX_MPS2_QEMU_IAR_GCCdemo,which is compiled with GCC. The IAR ports are not built by CI at all, which is how this
slipped through. Adding an assembler-only syntax check for these ports would be a
separate, larger change — happy to look at it if you think it is worth doing.
Relates to #1480.