Re: [Patch, avr] Place progmem.data from avr-libc above other progmem

[Senthil Kumar Selvaraj <senthil_kumar dot selvaraj at atmel dot com>]

Nick Clifton writes:

> Hi Senthil,
>
>>   avr-libc's vfprint handling uses a lookup table located in flash
>>   if float format specifiers are involved. If user code also has
>>   lots of flash data (in section .progmem.data), then
>>   avr-libc's progmem data gets pushed beyond the 64 K word limit. The
>>   avr-libc code doesn't expect this to happen and vfprintf stops working correctly.
>
> Doesn't this indicate a problem with the linker - in that it does not
> report to the user that the code is broken and that accesses above the 64k
> limit are being made without use of the memx address space modifier ?

Do you mean reloc processing should have detected the overflow? In
this case, the offending value is loaded into two 8 bit regs, using the
lo8 and hi8 modifiers, resulting in R_AVR_LO8_LDI_NEG and
R_AVR_HI8_LDI_NEG relocs. I guess if there was a single reloc that
handled the value, we could have detected overflow, AVR has only 8 bit
registers though, so I can't see a way to do this. Am I missing something?

>
>>   User code can easily use the  memx address space modifier to
>>   seamlessly access flash data above 64K. The avr-libc project doesn't
>>   use address space modifiers yet, and the lookup logic itself is
>>   written in assembly, so I guess it is not going to gain this ability
>>   soon.
>
> This sounds like a potential long term problem.  If the libc sources are
> not being maintained and improved, then things are only going to get worse 
> in the long run.

avr-libc is getting bugfixes and device support updates. It does try to
be backwards compatible with older gcc versions though, which makes it
kinda tricky to introduce code that exploits new features like __memx. I'm
planning to work on this next, FWIW.
>
>>   Given these constraints, this patch places progmem.data coming from
>>   avr-libc's routines above other progmem.data, increasing the
>>   likelyhood that it gets placed below the 64 K limit. If this is ok,
>
> Sure - it's OK, although it will not help in the situation where the user
> is using their own linker scripts...

Agreed, the patch isn't really a fix, it's a workaround to make sure things
aren't broken out of the box.

>
> Also, the new statements should be enclosed in ${RELOCATING+ ... } just
> like the other statements in that section.

Sorry, I'd missed that.
>
>>   could someone commit please? 
>
> Done.
>
>> I don't have commit access.
>
> Would you like to change this ?  You have submitted enough patches now
> that it is obvious that you know what you are doing.  So if you would to
> apply for commit access (subject to the usual maintainer review of course),
> then please fill out the form here.  You can use my name as the approver:
>
>   https://sourceware.org/cgi-bin/pdw/ps_form.cgi

Done. Thanks Nick!

Regards
Senthil