eyalroz / printf

Tiny, fast(ish), self-contained, fully loaded printf, sprinf etc. implementation; particularly useful in embedded systems.
MIT License
402 stars 50 forks source link

Incorrect size assumption for size_t on 16-bit target (msp430). #168

Closed geurtv closed 1 month ago

geurtv commented 10 months ago

As noted in README.md:

The implementation currently assumes each of intmax_t, signed size_t, and ptrdiff_t has the same size as long int or as long long int. If this is not the case for your platform, please open an issue.

With msp430-elf-gcc for the 16-bit msp430 microcontroller sizeof(size_t) == 2, which is the same as int and not as long (sizeof(long) == 4). A change like this should be portable and fixes the issue (intmax_t and ptrdiff_t will need similar changes):

flags |= sizeof(size_t) <= sizeof(int) ? FLAGS_INT : sizeof(size_t) == sizeof(long) ? FLAGS_LONG : FLAGS_LONG_LONG;
eyalroz commented 10 months ago

So, you're saying long is not representable in a single machine register? Are you 100% sure this is the case? That's a very weird choice for a platform's C ABI.

geurtv commented 10 months ago

Yes, that's exactly how it is, and I wouldn't expect this to be too uncommon for 16-bit microcontrollers. int (and short) will be the same size as the cpu register size, so 16-bit. Then long will be 32-bit and long long 64-bit (which gcc actually supports). For msp430 gcc has two addressing mode options: -msmall and -mlarge. With -msmall it uses 16-bit pointers and 16-bit size_t, with -mlarge it uses 20-bit pointers and 32-bit size_t (which should already work). The msp430 variant I'm using has 16k flash and 4k ram, so I use -msmall.

Btw, intmax_t already works as it's long long. For -msmall it's (only) size_t and ptrdiff_t that don't work.

eyalroz commented 10 months ago

Ok, if you say so... I'll try to find the time to consider your suggestion and check the code for potential impact of this situation.

However, as you may know, there's war now, and demonstrations against it, and I've already been arrested at one of those, and there are some personal issues, and my job etc., so it may take a bit of time. If I haven't done anything within... 3 weeks from now, please ping me.

eyalroz commented 9 months ago

Please check out the development branch and let me know if everything works for you.

geurtv commented 9 months ago

That does the trick, thanks! Note that ptrdiff_t needs similar treatment.

eyalroz commented 9 months ago

Right... can you recheck?

geurtv commented 9 months ago

Yes, that now works for both size_t and ptrdiff_t.