espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.62k stars 7.28k forks source link

Uninitialized DRAM_ATTR memory ends up in .data instead of .bss (IDFGH-10368) #11626

Open chenlijun99 opened 1 year ago

chenlijun99 commented 1 year ago

Answers checklist.

IDF version.

v5.0.2

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32-S3-DevKitC-1 v1.1

Power Supply used.

USB

What is the expected behavior?

Even with DRAM_ATTR if the object with static storage duration is not expliciltly initialized, it should be placed in .bss.

What is the actual behavior?

Given the code

static DRAM_ATTR int array[100];

if I run idf.py size I can see that without DRAM_ATTR it ends up in .bss, while with DRAM_ATTR it ends up in .data.

Steps to reproduce.

  1. Step
  2. Step
  3. Step ...

Debug Logs.

No response

More Information.

No response

igrr commented 1 year ago

That's true, but do you happen to know any method stop achieve this? Since the section attribute overrides the original section, there's no information available at linking stage to decide where the variable should go — .data or .bss.

It's rarely an issue, though, as DRAM_ATTR is mostly used for the purpose of moving constant arrays (which would otherwise be in .rodata, i.e. Flash) into .data. Do you have an actual use case where you must use DRAM_ATTR on an empty array?

chenlijun99 commented 1 year ago

That's true, but do you happen to know any method stop achieve this? Since the section attribute overrides the original section, there's no information available at linking stage to decide where the variable should go — .data or .bss.

May be not developer-friendly, but what about having another attribute? E.g. Having something like DRAM_ATTR_BSS?

It's rarely an issue, though, as DRAM_ATTR is mostly used for the purpose of moving constant arrays (which would otherwise be in .rodata, i.e. Flash) into .data. Do you have an actual use case where you must use DRAM_ATTR on an empty array?

My use case is to statically allocate DMA-capable memory buffers (for LVGL) using DMA_ATTR which I found uses DRAM_ATTR under the hood. But now that I think perhaps that is not required, assuming the whole DRAM is DMA-capable and assuming that the linker won't put my memory buffer in PSRAM (which it shouldn't unless I use EXT_RAM_BSS_ATTR). But I like to have DMA_ATTR to explicitly state the property that that piece of memory should have. Who knows, what if the next ESP MCU doesn't have full DMA-capable DRAM?

igrr commented 1 year ago

I'm afraid DRAM_ATTR_BSS could be error prone, for example someone might copy-paste the code with DRAM_ATTR_BSS in it and then add a non-zero initializer; then have to troubleshoot why the initializer is not having effect. Compared to this, a slightly larger binary size seems less of an issue.

But I like to have DMA_ATTR to explicitly state the property that that piece of memory should have.

That's a good idea to make your code future proof. I think the best I can suggest for now is to add assert(esp_ptr_dma_capable(array)) in your code so that at least you will notice if something drastic changes in the memory architecture of a hypothetical future chip.

chenlijun99 commented 1 year ago

Whao, thanks for the pointer. Wasn't aware of esp_ptr_dma_capable(). I agree that DRAM_ATTR_BSS may be quite error-prone, in a very subtle way. esp_ptr_dma_capable() would OTOH be very explicit in case of error.

Then, at least for me, the problem is solved. Feel free to close this issue!

RiccardoKM commented 1 year ago

Hello, I would be very glad to have this feature implemented.

Besides of buffers, static task stacks as well should be allocated in DRAM. In my case I have more than 40kB of flash memory wasted for this issue and I'm pretty sure some of the other 27kB I see in the .data segment from ESP libraries in my project are not initialized as well. In a project which has a 500kB size it makes up almost 10% of the binary.

Concerning the fact that _"DRAM_ATTRBSS could be error prone": ESP SDK already have some confusion about .data and .bss; as an example EXT_RAM_ATTR goes silently to .bss (to my disappointment when I discovered it, if I may). I think having DRAM_BSS_ATTR, or even better DRAM_UNINITIALIZED_ATTR (or something similar), would be much clearer and less error prone than what is already in place, back compatible (plain DRAM_ATTR may stay there) and would save some valuable binary size and flashing time.

Same considerations apply to IRAM_ATTR, if someone else might need it.

In case ESP developers end up deciding not to introduce this fix, may I ask a tip to modify #define DRAM_ATTR _SECTION_ATTR_IMPL(".dram1", __COUNTER__) in order to land some memory in DRAM .bss just in my project. Any help would be really appreciated :) Thanks in advance.

BitwiseMaster commented 6 months ago

I have the same issue as the OP. I define a buffer with DMA_ATTR to use for SPI transactions and it ends up in .data instead of .bss

To me this is an issue since I plan to use OTA at some point, so I'd like to keep the binarys as small as possible.

I can add this ugly hack so it ends up in .bss: #undef DMA_ATTR #define DMA_ATTR WORD_ALIGNED_ATTR DMA_ATTR uint8_t spi_buffer[4096];

which keeps the DMA_ATTR tag and the 4 byte alignment at least, but I really think these uninitialized buffers should end up in .bss