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

Bring linker scripts in sync with pico-sdk original. #16

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mvds00
Copy link
Contributor

@mvds00 mvds00 commented Nov 9, 2023

The linker script apparently evolved in pico-sdk. This patch updates the linker script in picowota to match the original script (memmap_default.ld) as closely as possible.

This will make it easier to spot issues as either project evolves.

A related goal is to have some way of using the pico-sdk linker scripts as a template for the linker scripts in e.g. picowota (see templated linker scripts, issue #398 of pico-sdk).

…eneration

The objcopy step that updates sections with binary data failed because of an error with sections now marked NOLOAD instead of COPY.

The error disappeared with moving section .uninitialized_data to near .ram_vector_table, which somehow triggered adding a section that reserved a piece of RAM (which is fine) with reference to a flash area (which is strange).

Note that these uninitialized sections are explicitly requested by __uninitialized_ram() directives:

pico-sdk/src/rp2_common/pico_platform/include/pico/platform.h:
    #define __uninitialized_ram(group) __attribute__((section(".uninitialized_data." #group))) group

Their use is to keep ram uninitialized on purpose, to collect entropy (for RNG) or data left over from before reset.
Copy link
Owner

@usedbytes usedbytes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm uneasy about merging this without understanding what's going on - especially as it immediately diverges the linker script again from the pico-sdk one

@@ -142,10 +142,15 @@ SECTIONS
__binary_info_end = .;
. = ALIGN(4);

.ram_vector_table (NOLOAD): {
.ram_vector_table (NOLOAD): {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you added a space to the start of this line in both linker scripts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, I aligned it with the other entries while I was at it.

@mvds00
Copy link
Contributor Author

mvds00 commented Nov 12, 2023

I'm uneasy about merging this without understanding what's going on - especially as it immediately diverges the linker script again from the pico-sdk one

You're referring only to ab7cef8 here, I guess.

Let me elaborate a bit.

The change itself is trivial and doesn't impact our standalone or picowota builds in a negative way. It is a rearrangement of sections placed in RAM with no (known) negative impact. AFAIK the only hard requirement could be that .ram_vector_table comes first, which is still honoured. I will attempt to get this change through with pico-sdk as well (but for them there is no downside of not changing it, see below).

The order of sections was:

.ram_vector_table { ... } > RAM
.data { ... } > RAM AT> FLASH
.uninitialized_data { ... } > RAM

Which we had to rearrange to:

.ram_vector_table { ... } > RAM
.uninitialized_data { ... } > RAM
.data { ... } > RAM AT> FLASH

Without this rearrangement, we see something strange popping up when using objcopy on the ELF when generating another ELF, but not when outputting -Obinary. So this only affects use cases like picowota and not regular pico-sdk builds. The issue seems to be that we get a section .uninitialized_data with VMA in RAM (expected) and LMA in flash (unexpected, because there is nothing in flash to load). The relevant output of objdump is:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
...
 15 .ram_vector_table 000000c0  20000000  20000000  00060000  2**2
                  ALLOC
 16 .data         00001224  200000c0  100a010c  0005a0c0  2**4
                  CONTENTS, ALLOC, LOAD, CODE
 17 .uninitialized_data 00000020  200012e8  100a1330  0005b2e8  2**3
                  ALLOC

The unexpected value here is LMA of .uninitialized_data of 100a1330.

Re-arranging gives:

 15 .ram_vector_table 000000c0  20000000  20000000  000d0000  2**2
                  ALLOC
 16 .uninitialized_data 00000020  200000c0  200000c0  000d0000  2**3
                  ALLOC
 17 .data         00001224  200000e0  100a010c  0005a0e0  2**4
                  CONTENTS, ALLOC, LOAD, CODE

Where we see the expected LMA .uninitialized_data for a RAM only section. It very much looks like the AT>FLASH is somehow sticky; and indeed adding AT>FLASH to the re-arranged .uninitialized_data leads to the same issue.

The actual issue may be that objcopy is confused. Simply issuing objcopy input.elf output.elf emits the following:

arm-none-eabi-objcopy: output.elf: section `.ram_vector_table' can't be allocated in segment 9
LOAD: .uninitialized_data .ram_vector_table .bss

And comparing input.elf and output.elf sheds a bit more light on the issue, when we compare the last segment (with index 9):

input.elf:
    LOAD off    0x00060000 vaddr 0x20000000 paddr 0x20000000 align 2**12
         filesz 0x00000000 memsz 0x00031f0c flags rw-

output.elf:
    LOAD off    0x0005f2e8 vaddr 0x200012e8 paddr 0x100a1330 align 2**12
         filesz 0x00000000 memsz 0x0ff90bdc flags rw-

Memsz seems to indicate some internal objcopy issue here, as its value is off by a lot. This seems to be a relevant equation: 0x0ff90bdc = 0x20031f0c - 0x100a1330; memsz seems to be calculated as end of allocated RAM, minus LMA of .uninitialized_data, where VMA should have been used (which should normally be equal to LMA).

For this I used GNU objdump (GNU Tools for Arm Embedded Processors 8-2018-q4-major) 2.31.51.20181213 and GNU objcopy (GNU Tools for Arm Embedded Processors 8-2018-q4-major) 2.31.51.20181213. The issue with objcopy was also seen using GNU objcopy (GNU Tools for Arm Embedded Processors 11-2022-q3-update) 2.32.0.20190703.

@mvds00
Copy link
Contributor Author

mvds00 commented Nov 17, 2023

@usedbytes I think this is actually a bug in the original linker script. The AT> FLASH is indeed sticky (by design, it's documented) and is inherited by subsequent >RAM sections. This then generates the bogus .uninitialized_data section with LMA != VMA. See the explanation in the related pico-sdk issue/PR.

@mvds00
Copy link
Contributor Author

mvds00 commented Jan 16, 2024

FYI: the equivalent fix was recently merged in develop of pico-sdk, see raspberrypi/pico-sdk#1539

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants