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

code relocation generating different memory layout cause user mode not working #17038

Closed
wentongwu opened this issue Jun 25, 2019 · 4 comments · Fixed by #17185
Closed

code relocation generating different memory layout cause user mode not working #17038

wentongwu opened this issue Jun 25, 2019 · 4 comments · Fixed by #17185
Assignees
Labels
area: Linker Scripts area: Memory Protection area: Userspace Userspace bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@wentongwu
Copy link
Contributor

wentongwu commented Jun 25, 2019

Describe the bug
In the magic building process for user mode, there are different memory layout as below log show.
The map file of building zephyr_prebuilt.elf is:

 .text.sedi_pm_pmu2nvic_isr
                0x0000000060011188        0xa libbsp_sedi_arm.a(pm.c.obj)
                0x0000000060011188                sedi_pm_pmu2nvic_isr
 *(SORT_BY_ALIGNMENT(.gnu.linkonce.t.*))
 *(SORT_BY_ALIGNMENT(.glue_7t))
 .glue_7t       0x0000000060011192        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.glue_7))
 .glue_7        0x0000000060011192        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.vfp11_veneer))
 .vfp11_veneer  0x0000000060011192        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.v4_bx))
 .v4_bx         0x0000000060011192        0x0 linker stubs
                0x0000000060011192                _priv_stacks_text_area_start = .
 *(SORT_BY_ALIGNMENT(.priv_stacks.text*))
 *fill*         0x0000000060011192        0x2 

And ztest_thread_stack located in 0x60030000 from below log and we can also find it from map file.
gen_kobject_list.py: symbol 'ztest_thread_stack' at 0x60030000 contains 1 object(s)

The map file of building priv_stacks_prebuilt.elf is:

.text.sedi_pm_pmu2nvic_isr
                0x0000000060011188        0xa libbsp_sedi_arm.a(pm.c.obj)
                0x0000000060011188                sedi_pm_pmu2nvic_isr
 *fill*         0x0000000060011192        0x6 
 .text.sedi_pm_pmu2nvic_isr.__stub
                0x0000000060011198       0x68 linker stubs
 *(SORT_BY_ALIGNMENT(.gnu.linkonce.t.*))
 *(SORT_BY_ALIGNMENT(.glue_7t))
 .glue_7t       0x0000000060011200        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.glue_7))
 .glue_7        0x0000000060011200        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.vfp11_veneer))
 .vfp11_veneer  0x0000000060011200        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.v4_bx))
 .v4_bx         0x0000000060011200        0x0 linker stubs
                0x0000000060011200                _priv_stacks_text_area_start = .

And ztest_thread_stack located in 0x60050000 from below log and we can also find it from map file.
gen_priv_stacks.py: symbol 'ztest_thread_stack' at 0x60050000 contains 1 object(s)

So that difference will cause the wrong hash table in priv_stacks_hash.c because of the different .rodata .noinit .data sections in the same one sram region and it will cause the user mode doesn't work.

@wentongwu wentongwu added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Memory Protection labels Jun 25, 2019
@wentongwu wentongwu self-assigned this Jun 25, 2019
@wentongwu
Copy link
Contributor Author

wentongwu commented Jun 27, 2019

From above map file, we can find there is an additional " .text.sedi_pm_pmu2nvic_isr.__stub" in the process of building priv_stacks_prebuilt.elf, but there is no this __stub for building zephyr_prebuilt.elf. So the random __stub generated by compiler causes the different size of .text section in the two process, and then different start address of .rodata, .noinit, .data sections(maybe sometimes the mpu align can cause some sections have same start address, but not always). So in priv_stacks_hash.c, the hash key(stack address) is not the final stack address.

@wentongwu
Copy link
Contributor Author

wentongwu commented Jun 27, 2019

@andrewboie PR #17042 is a workaround for this issue , and so far I haven't found the reason why compiler generate this __stub. If found it, I will remove this workaround.

wentongwu added a commit to wentongwu/zephyr that referenced this issue Jun 28, 2019
Code relocation causes comipler randomly generates _stub that will
cause different memory layout in the magic building process for user
mode. And the different memory layout will generate wrong hash table
in priv_stacks_hash.c and it will cause user mode doesn't work.

Align sram .text section when userspace and code relocation enabled
to generate same .rodata, .data and .bss sections in sram region.
User can adjust Kconfig RELOCATION_SRAM_TEXT_ALIGN_SIZE to make less
wasting memory.

Fixes: zephyrproject-rtos#17038.

Signed-off-by: Wentong Wu <[email protected]>
@wentongwu
Copy link
Contributor Author

wentongwu commented Jun 28, 2019

Ok, let's move the discussion here. The extra stubs are comprised of veneer functions, because the distance between caller and callee is too far, linker will automatically generate and insert veneer functions. Look at the map file again,

The map file of building priv_stacks_prebuilt.elf is:

 .text.sedi_pm_pmu2nvic_isr
                0x000000006001117c        0xa libbsp_sedi_arm.a(pm.c.obj)
                0x000000006001117c                sedi_pm_pmu2nvic_isr
 *fill*         0x0000000060011186        0x2
 .text.sedi_pm_pmu2nvic_isr.__stub
                0x0000000060011188       0x68 linker stubs
 *(SORT_BY_ALIGNMENT(.gnu.linkonce.t.*))
 *(SORT_BY_ALIGNMENT(.glue_7t))
 .glue_7t       0x00000000600111f0        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.glue_7))
 .glue_7        0x00000000600111f0        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.vfp11_veneer))
 .vfp11_veneer  0x00000000600111f0        0x0 linker stubs
 *(SORT_BY_ALIGNMENT(.v4_bx))
 .v4_bx         0x00000000600111f0        0x0 linker stubs
                0x00000000600111f0                _priv_stacks_text_area_start = .
 *(SORT_BY_ALIGNMENT(.priv_stacks.text*))
                0x00000000600111f0                _priv_stacks_text_area_end = .
                0x00000000600001e8                _priv_stacks_text_area_used = (_priv_stacks_text_area_end - _priv_stacks_text_area_start)
                0x00000000600111f0                PROVIDE (z_priv_stack_find = .)
                0x00000000600112f0                . = MAX (., (_priv_stacks_text_area_start + 0x100))
 *fill*         0x00000000600111f0      0x100
                0x0000000000000001                ASSERT ((0x100 >= _priv_stacks_text_area_used), The configuration system has incorrectly set

The map file of building zephyr_prebuilt.elf is:

.text.sedi_pm_pmu2nvic_isr
               0x000000006001117c        0xa libbsp_sedi_arm.a(pm.c.obj)
               0x000000006001117c                sedi_pm_pmu2nvic_isr
*(SORT_BY_ALIGNMENT(.gnu.linkonce.t.*))
*(SORT_BY_ALIGNMENT(.glue_7t))
.glue_7t       0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.glue_7))
.glue_7        0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.vfp11_veneer))
.vfp11_veneer  0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.v4_bx))
.v4_bx         0x0000000060011186        0x0 linker stubs
               0x0000000060011186                _priv_stacks_text_area_start = .
*(SORT_BY_ALIGNMENT(.priv_stacks.text*))
*fill*         0x0000000060011186        0x2
.priv_stacks.text
               0x0000000060011188       0x28 zephyr/priv_stacks_hash_renamed.o
               0x0000000060011188                z_priv_stack_find
.priv_stacks.text.__stub
               0x00000000600111b0       0x68 linker stubs
               0x0000000060011218                _priv_stacks_text_area_end = .
               0x000000006000027a                _priv_stacks_text_area_used = (_priv_stacks_text_area_end - _priv_stacks_text_area_start)
               [!provide]                        PROVIDE (z_priv_stack_find = .)
               0x0000000060011286                . = MAX (., (_priv_stacks_text_area_start + 0x100))
*fill*         0x0000000060011218       0x6e
               0x0000000000000001                ASSERT ((0x100 >= _priv_stacks_text_area_used), The configuration system has incorrectly set

The map file of building zephyr.elf is:

.text.sedi_pm_pmu2nvic_isr
               0x000000006001117c        0xa libbsp_sedi_arm.a(pm.c.obj)
               0x000000006001117c                sedi_pm_pmu2nvic_isr
*(SORT_BY_ALIGNMENT(.gnu.linkonce.t.*))
*(SORT_BY_ALIGNMENT(.glue_7t))
.glue_7t       0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.glue_7))
.glue_7        0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.vfp11_veneer))
.vfp11_veneer  0x0000000060011186        0x0 linker stubs
*(SORT_BY_ALIGNMENT(.v4_bx))
.v4_bx         0x0000000060011186        0x0 linker stubs
               0x0000000060011186                _priv_stacks_text_area_start = .
*(SORT_BY_ALIGNMENT(.priv_stacks.text*))
*fill*         0x0000000060011186        0x2
.priv_stacks.text
               0x0000000060011188       0x28 zephyr/priv_stacks_hash_renamed.o
               0x0000000060011188                z_priv_stack_find
               0x00000000600111b0                _priv_stacks_text_area_end = .
               0x0000000060000212                _priv_stacks_text_area_used = (_priv_stacks_text_area_end - _priv_stacks_text_area_start)
               0x0000000060011286                . = MAX (., (_priv_stacks_text_area_start + 0x100))
*fill*         0x00000000600111b0       0xd6
               0x0000000000000001                ASSERT ((0x100 >= _priv_stacks_text_area_used), The configuration system has incorrectly set
'CONFIG_PRIVILEGED_STACK_TEXT_AREA' to
256, which is not big enough. You must
through Kconfig either disable 'CONFIG_USERSPACE', or set
'CONFIG_PRIVILEGED_STACK_TEXT_AREA' to a value larger than
256 .)
               0x0000000060011286                _kobject_text_area_start = .
*(SORT_BY_ALIGNMENT(.kobject_data.text*))
*fill*         0x0000000060011286        0x2
.kobject_data.text
               0x0000000060011288       0x5c zephyr/kobject_hash_renamed.o
               0x0000000060011288                z_object_find
               0x0000000060011288                z_object_gperf_find
               0x00000000600112c0                z_object_gperf_wordlist_foreach
               0x00000000600112c0                z_object_wordlist_foreach
*fill*         0x00000000600112e4        0x4
.kobject_data.text.__stub
               0x00000000600112e8       0x68 linker stubs
               0x0000000060011350                _kobject_text_area_end = .
               0x0000000060011386                . = (. + (0x100 - (_kobject_text_area_end - _kobject_text_area_start)))
*fill*         0x0000000060011350       0x36
               0x0000000060011386                _image_text_end = .

From above fragments, we know even though the stubs call different names (.text.sedi_pm_pmu2nvic_isr.__stub, .priv_stacks.text.__stub, .kobject_data.text.__stub), but they contain the same veneer functions and have same size(0x68 Bytes), so it seems linker will put the linker stubs(veneer functions) at the end of .text sections(right after the last instruction), and because priv_stacks-text.ld and kobject-text.ld hold enough memory to contain these linker stubs. But when generating priv_stacks_prebuilt.elf, these linker stubs don't been put in these holding memory, so it will cause the different memory layout. So my current idea will be like below, and checked the three maps with below change, they have same memory layout. I will verify below change when get the setup at Monday. If @andrewboie @nashif @jocelyn-li have any concerns about below changes, please let me know.

diff --git a/include/arch/arm/cortex_m/scripts/linker.ld b/include/arch/arm/cortex_m/scripts/linker.ld
index fb9e9f4642..239b2e6a64 100644
--- a/include/arch/arm/cortex_m/scripts/linker.ld
+++ b/include/arch/arm/cortex_m/scripts/linker.ld
@@ -202,6 +202,10 @@ SECTIONS
     SECTION_PROLOGUE(_TEXT_SECTION_NAME_2,,)
        {
        _image_text_start = .;
+
+#include <linker/priv_stacks-text.ld>
+#include <linker/kobject-text.ld>
+
        *(.text)
        *(".text.*")
        *(.gnu.linkonce.t.*)
@@ -212,9 +216,6 @@ SECTIONS
         */
        *(.glue_7t) *(.glue_7) *(.vfp11_veneer) *(.v4_bx)
 
-#include <linker/priv_stacks-text.ld>
-#include <linker/kobject-text.ld>
-
        } GROUP_LINK_IN(ROMABLE_REGION)
 
        _image_text_end = .;

Update: I do the test with this change, code relocation for user mode works well.

@wentongwu
Copy link
Contributor Author

wentongwu commented Jun 28, 2019

@andrewboie Another issue I want to talk with you is that when code relocation enabled, there will be some other regions to hold kernel objects(like k_mutex, k_sem, etc), but in elf_helper.py, it will do the address check like below code, only in SRAM region, so kernel objects in other regions will be discarded. There is a method which is to add symbols in these regions to mark the start and end of the memory containing kernel objects, and then add the check here. But I want to know if I delete below check logic, is there any other side effect? Because, run time we will check if there is hash key in the hash table, even there is no below check, other intend and unintended non-kernel object address will also be dropped when these addresses being passed to kernel space. If anything wrong, please let me know.

497             _, user_ram_allowed = kobjects[ko.type_obj.name]
498             if (not user_ram_allowed and
499                     (addr < kram_start or addr >= kram_end) and
500                     (addr < krom_start or addr >= krom_end)):
501 
502                 self.debug_die(die,
503                                "object '%s' found in invalid location %s"
504                                % (name, hex(addr)))
505                 continue

wentongwu added a commit to wentongwu/zephyr that referenced this issue Jul 1, 2019
When code relocation enabled, there will be serval regions holding
text. And then there will be function call between these .text
regions, when distance between caller and callee is too far, linker
will automatically generate and insert veneer functions. And these
veneer functions will be located right after the last instruction
in the .text region by the linker. So these code will be put in the
memory reserved for priv_stacks text and kobject text if they don't
consume all the reserved memory. Or the veneer functions will be put
before the reserved memory if there isn't code in the reserved
memory. And then in the user mode building process, there will be
different memory layout and it will cause usr mode not working.
And moving the memory reserved for priv_stacks text and kobject text
at the beginning of .text will avoid above problem. The detailed
analysis for this issue can be found on Github issue zephyrproject-rtos#17038.

Fixes: zephyrproject-rtos#17038.

Signed-off-by: Wentong Wu <[email protected]>
nashif pushed a commit that referenced this issue Jul 18, 2019
When code relocation enabled, there will be serval regions holding
text. And then there will be function call between these .text
regions, when distance between caller and callee is too far, linker
will automatically generate and insert veneer functions. And these
veneer functions will be located right after the last instruction
in the .text region by the linker. So these code will be put in the
memory reserved for priv_stacks text and kobject text if they don't
consume all the reserved memory. Or the veneer functions will be put
before the reserved memory if there isn't code in the reserved
memory. And then in the user mode building process, there will be
different memory layout and it will cause usr mode not working.
And moving the memory reserved for priv_stacks text and kobject text
at the beginning of .text will avoid above problem. The detailed
analysis for this issue can be found on Github issue #17038.

Fixes: #17038.

Signed-off-by: Wentong Wu <[email protected]>
wentongwu added a commit to wentongwu/zephyr that referenced this issue Jul 31, 2019
When code relocation enabled, there will be serval regions holding
text. And then there will be function call between these .text
regions, when distance between caller and callee is too far, linker
will automatically generate and insert veneer functions. And these
veneer functions will be located right after the last instruction
in the .text region by the linker. So these code will be put in the
memory reserved for priv_stacks text and kobject text if they don't
consume all the reserved memory. Or the veneer functions will be put
before the reserved memory if there isn't code in the reserved
memory. And then in the user mode building process, there will be
different memory layout and it will cause usr mode not working.
And moving the memory reserved for priv_stacks text and kobject text
at the beginning of .text will avoid above problem. The detailed
analysis for this issue can be found on Github issue zephyrproject-rtos#17038.

Fixes: zephyrproject-rtos#17038.

Signed-off-by: Wentong Wu <[email protected]>
nashif pushed a commit that referenced this issue Aug 15, 2019
When code relocation enabled, there will be serval regions holding
text. And then there will be function call between these .text
regions, when distance between caller and callee is too far, linker
will automatically generate and insert veneer functions. And these
veneer functions will be located right after the last instruction
in the .text region by the linker. So these code will be put in the
memory reserved for priv_stacks text and kobject text if they don't
consume all the reserved memory. Or the veneer functions will be put
before the reserved memory if there isn't code in the reserved
memory. And then in the user mode building process, there will be
different memory layout and it will cause usr mode not working.
And moving the memory reserved for priv_stacks text and kobject text
at the beginning of .text will avoid above problem. The detailed
analysis for this issue can be found on Github issue #17038.

Fixes: #17038.

Signed-off-by: Wentong Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment