Hi, David, Hi Ammar
From: Ammar Faizi
Sent: 30 August 2023 15:41
On 8/28/23 4:51 AM, David Laight wrote:
I just found a(nother) clang bug: int f(void) { return "a"[2]; } compiles to just a 'return'.
I don't think that's a bug. It's undefined behavior due to an out-of-bound read. What do you expect it to return?
I was actually expecting a warning/error if it didn't just read the byte after the end of the string.
Just silently doing nothing didn't seem right for a modern compiler.
godbolt.org uses 'orange' color (see its right top) to indicate a warning, it does report a warning (see Output label in right bottom) when we access the byte after the end of the string, but gcc doesn't report a warning ;-)
int test_outofbound(void) { return "a"[10]; }
see the last test case in https://godbolt.org/z/9jY4xoWrT
But it is safe if we use the trick like David used for the above __atoi() macro:
if (str[0]) { /* always make sure the index is safe and stop at the end of string */ if (str[1]) { if (str[2]) { .... } } }
We also need this style of checking for the delta logic in __atoi_add(). have randomly tried different clang and gcc versions, seems all of them work correctly, but the compiling speed is not that good if we want to support the worst cases like "((0x900000 + 0x0f0000) + 5)", the shorter one "((0x900000+0x0f0000)+5)" is used by ARM+OABI (not supported by nolibc currently), therefore, we can strip some tailing branches but it is either not that fast, of course, the other architectures/variants can use faster __atoi_add() versions with less branches and without hex detection, comparison and calculating.
As a short summary, the compling speed should not be a big problem for most of the architectures but to support the worst case __NR_*, the compiling speed will be very slow (for these cases, perhaps we can use a C version of atoi_add() instead or convert them to a more generic style: (6000 + 111), no hex and no multiple add), and the .i output is a little ugly and the debugging may be also a problem: for we can not assume the kernel developers always define a short and a simple style of __NR_* as we expected. So, the __nrtoi() requires more work, let's delay the whole RFC patchset and work on some more urgent tasks at first as suggested by Willy, but David's NR_toi() prototype is really a very valuable base for future work, really appreciate, I will back to this discussion if have any new progress, thanks!
Thanks very much, Zhangjin
David