Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linker script: Vector table regression due to change in definition of _vector_end #47273

Closed
ABOSTM opened this issue Jul 4, 2022 · 7 comments · Fixed by #47309 or #47597
Closed

linker script: Vector table regression due to change in definition of _vector_end #47273

ABOSTM opened this issue Jul 4, 2022 · 7 comments · Fixed by #47309 or #47597
Assignees
Labels
area: Linker Scripts bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 Regression Something, which was working, does not anymore

Comments

@ABOSTM
Copy link
Collaborator

ABOSTM commented Jul 4, 2022

Describe the bug
A series of tests are now failed on STMicroelectronics test bench on STM32F0 series (Cortex M0):

  • tests/arch/arm/arm_interrupt/arch.interrupt.arm
  • tests/arch/arm/arm_interrupt/arch.interrupt.extra_exception_info
  • tests/arch/arm/arm_interrupt/arch.interrupt.no_optimizations
  • tests/arch/arm/arm_irq_advanced_features/arch.arm.irq_advanced_features
  • tests/arch/arm/arm_irq_vector_table/arch.arm.irq_vector_table
  • tests/drivers/adc/adc_api/drivers.adc
  • tests/drivers/counter/counter_basic_api/drivers.counter.basic_api
  • tests/drivers/dma/chan_blen_transfer/drivers.dma.chan_blen_transfer
  • tests/drivers/dma/loop_transfer/drivers.dma.loop_transfer
  • tests/drivers/gpio/gpio_api_1pin/peripheral.gpio.1pin
  • tests/kernel/interrupt/arch.interrupt
  • tests/kernel/profiling/profiling_api/kernel.common.profiling
  • tests/kernel/tickless/tickless_concept/kernel.tickless.concept
  • tests/kernel/timer/timer_api/kernel.timer.tickless
  • tests/subsys/pm/device_runtime_api/pm.device_runtime.api

Test/MCU is stuck when handling interrupts.

This is a regression introduced since merge of PR: #46769 "Try to fix IRQ vector table relocation"
and especially the move of line
KEEP(*(_IRQ_VECTOR_TABLE_SECTION_SYMS))
from arch/arm/core/aarch32/vector_table.ld to tests/arch/arm/arm_irq_vector_table/irq-vector-table.ld

See detailed analysis below.

To Reproduce
Steps to reproduce the behavior:

  1. west build -p auto -b nucleo_f091rc tests/arch/arm/arm_interrupt/
  2. west flash
  3. See error

Logs and console output
*** Booting Zephyr OS build zephyr-v3.1.0-985-gd497cca845e8 ***

Running TESTSUITE arm_interrupt
===================================================================
START - test_arm_null_pointer_exception
Skipped
 PASS - test_arm_null_pointer_exception in 0.1 seconds
===================================================================
START - test_arm_interrupt
Available IRQ line: 30
E: ***** HARD FAULT *****

Environment (please complete the following information):

  • OS: . Linux,
  • Toolchain Zephyr SDK
  • Commit SHA or Version used
@ABOSTM ABOSTM added bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 Regression Something, which was working, does not anymore area: Linker Scripts labels Jul 4, 2022
@ABOSTM
Copy link
Collaborator Author

ABOSTM commented Jul 4, 2022

Detailed analysis

Before the PR #46769, arch/arc/core/vector_table.ld , defines

_vector_start = .;
KEEP(*(.exc_vector_table))
KEEP(*(".exc_vector_table.*"))

KEEP(*(_IRQ_VECTOR_TABLE_SECTION_SYMS))

KEEP(*(.vectors))

_vector_end = .;

Thus _vector_end includes exception vector table as well as IRQ vector table.
This is confirmed by zephyr.map file that show
0x080000bc _vector_end = .
(0x080000bc is effectively the end of the whole vector table)

On the contrary, after the PR #46769,

_vector_start = .;
KEEP(*(.exc_vector_table))
KEEP(*(".exc_vector_table.*"))
KEEP(*(.vectors))

_vector_end = .;

Thus _vector_end doesn't include IRQ table, but only exception table,
this is confirmed by zephyr.map file that show
0x08000040 _vector_end = .
(0x08000040 is effectively the end of the exception vector table)

In other words, it is like patch changes the definition of symbol _vector_end
from whole table end to exception table end only.
And because this symbol is used in specific STM32F0 linker script

. += _vector_end - _vector_start;

this causes tests failures due to bad vector table.

@carlocaione
Is it intentional to change the definition of _vector_end ? (It may impact other code)
Shall we keep the new definition or revert to the old one ?

@carlocaione
Copy link
Collaborator

Is it intentional to change the definition of _vector_end ? (It may impact other code)

Not really. Why CI didn't catch this?

Shall we keep the new definition or revert to the old one ?

I don't want to revert that PR. Let me see if I can fix this in a proper way.

@carlocaione
Copy link
Collaborator

@ABOSTM can you try this patch since I don't have the hardware?

diff --git a/include/zephyr/linker/irq-vector-table-section.ld b/include/zephyr/linker/irq-vector-table-section.ld
index 6a07abc34e..a1947ef74d 100644
--- a/include/zephyr/linker/irq-vector-table-section.ld
+++ b/include/zephyr/linker/irq-vector-table-section.ld
@@ -2,3 +2,6 @@
 
 . = ALIGN(CONFIG_ARCH_IRQ_VECTOR_TABLE_ALIGN);
 KEEP(*(_IRQ_VECTOR_TABLE_SECTION_SYMS))
+
+/* On ARM this symbol must be after the IRQ vector table */
+_vector_end = .;

@ABOSTM
Copy link
Collaborator Author

ABOSTM commented Jul 5, 2022

@carlocaione thanks for your quick response.
Yes, 1st patch works, moving _vector_end from vector_table.ld to irq-vector-table-section.ld

About your 2nd post, I was not sure you request to test only the addition of _vector_end in irq-vector-table-section.ld
In doubts I tests it and it is working too.

@ABOSTM
Copy link
Collaborator Author

ABOSTM commented Jul 8, 2022

@carlocaione ,
I reopen this issue because despite the fix proposed in #47309
there is still one test failed: tests/arch/arm/arm_irq_vector_table
(all other tests are passed).
I am confused, I should have check all tests instead of only one 😕

With a passed tests, let's say tests/arch/arm/arm_interrupt , the map file shows:
0x080000bc _vector_end = .
fine, it looks like it uses definition from irq-vector-table-section.ld

But with failed test : tests/arch/arm/arm_irq_vector_table the map file shows:
0x08000040 _vector_end = .
This is not good, it looks like it uses definition from vector_table.ld

I tried to remove _vector_end from vector_table.ld and keep only the one from irq-vector-table-section.ld
but in this case, there is a linker error _vector_end is undefined, which confirms that in case of tests/arch/arm/arm_irq_vector_table the definition used is the one from vector_table.ld

I noticed that tests/arch/arm/arm_irq_vector_table has its own linker file
https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/arch/arm/arm_irq_vector_table/irq-vector-table.ld
shall we add _vector_end in this file ?
But it doesn't work, map file show 0x0800004c _vector_end = .

@ABOSTM ABOSTM reopened this Jul 8, 2022
@carlocaione
Copy link
Collaborator

there is still one test failed: tests/arch/arm/arm_irq_vector_table

That test is kind special in the sense that it is defining by itself the IRQ vector table.

With a passed tests, let's say tests/arch/arm/arm_interrupt , the map file shows:
0x080000bc _vector_end = .
fine, it looks like it uses definition from irq-vector-table-section.ld

But with failed test : tests/arch/arm/arm_irq_vector_table the map file shows:
0x08000040 _vector_end = .
This is not good, it looks like it uses definition from vector_table.ld

That's fine. _vector_end must not always have the same value, this is not a problem at all.

I noticed that tests/arch/arm/arm_irq_vector_table has its own linker file
https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/arch/arm/arm_irq_vector_table/irq-vector-table.ld
shall we add _vector_end in this file ?

Yes, the correct fix here is to add _vector_end = .; at the end of the tests/arch/arm/arm_irq_vector_table/irq-vector-table.ld file.

But it doesn't work, map file show 0x0800004c _vector_end = .

What do you mean it doesn't work? 0x0800004c is the correct address for this test.

@ABOSTM
Copy link
Collaborator Author

ABOSTM commented Jul 11, 2022

What do you mean it doesn't work? 0x0800004c is the correct address for this test.

I was expecting _vector_end to be always the size of the hardware vectors,
but ok, I understand that for tests purpose this is not the case for this specific test.
Then test is passed with the new fix ! !
I will submit a PR. Thanks

ABOSTM added a commit to ABOSTM/zephyr that referenced this issue Jul 11, 2022
Following implementation of commit 219d5b5,
and to complement commit 8c4f98d
it is also necessary define _vector_end in
test specific arm-irq-vector-table.ld

Fixes zephyrproject-rtos#47273

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
fabiobaltieri pushed a commit that referenced this issue Jul 11, 2022
Following implementation of commit 219d5b5,
and to complement commit 8c4f98d
it is also necessary define _vector_end in
test specific arm-irq-vector-table.ld

Fixes #47273

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Linker Scripts bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 Regression Something, which was working, does not anymore
Projects
None yet
2 participants