Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Eduardo Barthel
I have a large lua application that works fine with Lua 5.3 and Lua 5.4 that is shipped by my Linux package manager. However I've downloaded the Lua 5.4 sources and compiled myself and when using it I got a crash. I was intrigued because the sources of my system's Lua and the ones I downloaded were equal, just the compilation flags were different. So I decided to test each CFLAGS when compiling to see which one was effecting.

For instance my system compiles Lua 5.4 with the following flags:
gcc -std=gnu99 -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_LINUX -DLUA_USE_READLINE -march=native -O2 -pipe -fno-plt -fPIC -flto -g -D_FORTIFY_SOURCE=2

And I was manually compiling with the following flags:
gcc -DLUA_COMPAT_5_3 -DLUA_USE_LINUX -O2 -g

Changing the CFLAGS to the same CFLAGS used by my system my application didn't crash, then I ruled out each flag one by one, and I discovered the following:
Adding -D_FORTIFY_SOURCE=2 makes the crash go away with -O2.
Changing -O2 to -O1 (but not adding -D_FORTIFY_SOURCE=2) also makes the crash go away.

I've tried to use a different compiler, such as clang, and the crash still happens under the same conditions.
I've tried applying patches for all the known Lua 5.4 bugs, but the crash still happens under the same conditions.

And here is the full backtrace with symbols and source location:
https://pastebin.com/7ZAL1Edg
In that run -O2 -g flags were used, and I've used sources from the "v5.4.0-patch" branch for lua's github repository.

Unfortunately I don't have a small test case yet.

---
Eduardo Bart
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Eduardo Barthel
Okay, I've managed to isolate to a simple test case:

```
tostring(1.4999999999999999e-30)
```

This simple line crashes Lua 5.4 for me if it's not compiled with -D_FORTIFY_SOURCE=2 when using -O2.

```
lua5.4
Lua 5.4.0  Copyright (C) 1994-2020 Lua.org, PUC-Rio
> tostring(1.4999999999999999e-30)
Bus error (core dumped)
```

Em ter., 18 de ago. de 2020 às 15:04, Eduardo Bart <[hidden email]> escreveu:
I have a large lua application that works fine with Lua 5.3 and Lua 5.4 that is shipped by my Linux package manager. However I've downloaded the Lua 5.4 sources and compiled myself and when using it I got a crash. I was intrigued because the sources of my system's Lua and the ones I downloaded were equal, just the compilation flags were different. So I decided to test each CFLAGS when compiling to see which one was effecting.

For instance my system compiles Lua 5.4 with the following flags:
gcc -std=gnu99 -O2 -Wall -Wextra -DLUA_COMPAT_5_3 -DLUA_USE_LINUX -DLUA_USE_READLINE -march=native -O2 -pipe -fno-plt -fPIC -flto -g -D_FORTIFY_SOURCE=2

And I was manually compiling with the following flags:
gcc -DLUA_COMPAT_5_3 -DLUA_USE_LINUX -O2 -g

Changing the CFLAGS to the same CFLAGS used by my system my application didn't crash, then I ruled out each flag one by one, and I discovered the following:
Adding -D_FORTIFY_SOURCE=2 makes the crash go away with -O2.
Changing -O2 to -O1 (but not adding -D_FORTIFY_SOURCE=2) also makes the crash go away.

I've tried to use a different compiler, such as clang, and the crash still happens under the same conditions.
I've tried applying patches for all the known Lua 5.4 bugs, but the crash still happens under the same conditions.

And here is the full backtrace with symbols and source location:
https://pastebin.com/7ZAL1Edg
In that run -O2 -g flags were used, and I've used sources from the "v5.4.0-patch" branch for lua's github repository.

Unfortunately I don't have a small test case yet.

---
Eduardo Bart
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Luiz Henrique de Figueiredo
In reply to this post by Eduardo Barthel
The page https://stackoverflow.com/questions/13517526/difference-between-gcc-d-fortify-source-1-and-d-fortify-source-2
says:

"If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1
(gcc -O1) and above, checks that shouldn't change the behavior of
conforming programs are performed. With _FORTIFY_SOURCE set to 2, some
more checking is added, but some conforming programs might fail."

"some conforming programs might fail" caught my eye...
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Eduardo Barthel
One correction, the crash does not happen with clang compiler (I've mistakenly thought that I tested with it), but I'm sure it happens on GCC 10.1.0 for me.
I've nailed down the crash to this line in `tostringbuff`:

```
len = lua_number2str(buff, MAXNUMBER2STR, fltvalue(obj));
```

When compiling with -fsanitize=address in GCC I get this:

```
./lua -e "tostring(1.4999999999999999e-30)"
AddressSanitizer:DEADLYSIGNAL
=================================================================
==224356==ERROR: AddressSanitizer: BUS on unknown address 0x000000000000 (pc 0x7f660403ca61 bp 0x39be6c71fe61a3ef sp 0x7ffdbed5f3e0 T0)
    #0 0x7f660403ca61 in __vsnprintf_internal (/usr/lib/libc.so.6+0x7ea61)
    #1 0x7f660432e73d in __interceptor_vsnprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1647
    #2 0x7f660432e9ef in __interceptor_snprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1718
    #3 0x55f013c8f4c0 in tostringbuff.part.0.isra.0 lua/lobject.c:350
    #4 0x55f013c2cf9c in tostringbuff lua/lobject.c:346
    #5 0x55f013c2cf9c in addnum2buff lua/lobject.c:454
    #6 0x55f013c2d485 in luaO_pushvfstring lua/lobject.c:492
    #7 0x55f013c3ecb5 in lua_pushfstring lua/lapi.c:542
    #8 0x55f013c53cc7 in luaL_tolstring lua/lauxlib.c:870
    #9 0x55f013c54343 in luaB_tostring lua/lbaselib.c:479
````

I've tried placing "assert(buff != NULL)" on that line, but when doing this the crash simply vanishes away and the buff pointer seems valid. Thus I think this perhaps could be an issue with GCC optimizations.


Em ter., 18 de ago. de 2020 às 15:59, Luiz Henrique de Figueiredo <[hidden email]> escreveu:
The page https://stackoverflow.com/questions/13517526/difference-between-gcc-d-fortify-source-1-and-d-fortify-source-2
says:

"If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1
(gcc -O1) and above, checks that shouldn't change the behavior of
conforming programs are performed. With _FORTIFY_SOURCE set to 2, some
more checking is added, but some conforming programs might fail."

"some conforming programs might fail" caught my eye...
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Eduardo Barthel
I've been trying different optimizations flags in gcc and something very odd is going with GCC 10.1.0

-O2                                                      => crash
-O2 -D_FORTIFY_SOURCE=2          => no crash
-O2 -fno-partial-inlining                       => no crash
-O1                                                      => no crash
-O1 -fpartial-inlining                             => no crash

Different optimization combinations fixes the issue.

Em ter., 18 de ago. de 2020 às 16:28, Eduardo Bart <[hidden email]> escreveu:
One correction, the crash does not happen with clang compiler (I've mistakenly thought that I tested with it), but I'm sure it happens on GCC 10.1.0 for me.
I've nailed down the crash to this line in `tostringbuff`:

```
len = lua_number2str(buff, MAXNUMBER2STR, fltvalue(obj));
```

When compiling with -fsanitize=address in GCC I get this:

```
./lua -e "tostring(1.4999999999999999e-30)"
AddressSanitizer:DEADLYSIGNAL
=================================================================
==224356==ERROR: AddressSanitizer: BUS on unknown address 0x000000000000 (pc 0x7f660403ca61 bp 0x39be6c71fe61a3ef sp 0x7ffdbed5f3e0 T0)
    #0 0x7f660403ca61 in __vsnprintf_internal (/usr/lib/libc.so.6+0x7ea61)
    #1 0x7f660432e73d in __interceptor_vsnprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1647
    #2 0x7f660432e9ef in __interceptor_snprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1718
    #3 0x55f013c8f4c0 in tostringbuff.part.0.isra.0 lua/lobject.c:350
    #4 0x55f013c2cf9c in tostringbuff lua/lobject.c:346
    #5 0x55f013c2cf9c in addnum2buff lua/lobject.c:454
    #6 0x55f013c2d485 in luaO_pushvfstring lua/lobject.c:492
    #7 0x55f013c3ecb5 in lua_pushfstring lua/lapi.c:542
    #8 0x55f013c53cc7 in luaL_tolstring lua/lauxlib.c:870
    #9 0x55f013c54343 in luaB_tostring lua/lbaselib.c:479
````

I've tried placing "assert(buff != NULL)" on that line, but when doing this the crash simply vanishes away and the buff pointer seems valid. Thus I think this perhaps could be an issue with GCC optimizations.


Em ter., 18 de ago. de 2020 às 15:59, Luiz Henrique de Figueiredo <[hidden email]> escreveu:
The page https://stackoverflow.com/questions/13517526/difference-between-gcc-d-fortify-source-1-and-d-fortify-source-2
says:

"If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1
(gcc -O1) and above, checks that shouldn't change the behavior of
conforming programs are performed. With _FORTIFY_SOURCE set to 2, some
more checking is added, but some conforming programs might fail."

"some conforming programs might fail" caught my eye...
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Eduardo Barthel
I've used gcc 9.3.0 to compile the same sources with the same flags and the code runs fine with any cflags combination, so I am sure it's something related to GCC 10.1.0.

Em ter., 18 de ago. de 2020 às 16:44, Eduardo Bart <[hidden email]> escreveu:
I've been trying different optimizations flags in gcc and something very odd is going with GCC 10.1.0

-O2                                                      => crash
-O2 -D_FORTIFY_SOURCE=2          => no crash
-O2 -fno-partial-inlining                       => no crash
-O1                                                      => no crash
-O1 -fpartial-inlining                             => no crash

Different optimization combinations fixes the issue.

Em ter., 18 de ago. de 2020 às 16:28, Eduardo Bart <[hidden email]> escreveu:
One correction, the crash does not happen with clang compiler (I've mistakenly thought that I tested with it), but I'm sure it happens on GCC 10.1.0 for me.
I've nailed down the crash to this line in `tostringbuff`:

```
len = lua_number2str(buff, MAXNUMBER2STR, fltvalue(obj));
```

When compiling with -fsanitize=address in GCC I get this:

```
./lua -e "tostring(1.4999999999999999e-30)"
AddressSanitizer:DEADLYSIGNAL
=================================================================
==224356==ERROR: AddressSanitizer: BUS on unknown address 0x000000000000 (pc 0x7f660403ca61 bp 0x39be6c71fe61a3ef sp 0x7ffdbed5f3e0 T0)
    #0 0x7f660403ca61 in __vsnprintf_internal (/usr/lib/libc.so.6+0x7ea61)
    #1 0x7f660432e73d in __interceptor_vsnprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1647
    #2 0x7f660432e9ef in __interceptor_snprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1718
    #3 0x55f013c8f4c0 in tostringbuff.part.0.isra.0 lua/lobject.c:350
    #4 0x55f013c2cf9c in tostringbuff lua/lobject.c:346
    #5 0x55f013c2cf9c in addnum2buff lua/lobject.c:454
    #6 0x55f013c2d485 in luaO_pushvfstring lua/lobject.c:492
    #7 0x55f013c3ecb5 in lua_pushfstring lua/lapi.c:542
    #8 0x55f013c53cc7 in luaL_tolstring lua/lauxlib.c:870
    #9 0x55f013c54343 in luaB_tostring lua/lbaselib.c:479
````

I've tried placing "assert(buff != NULL)" on that line, but when doing this the crash simply vanishes away and the buff pointer seems valid. Thus I think this perhaps could be an issue with GCC optimizations.


Em ter., 18 de ago. de 2020 às 15:59, Luiz Henrique de Figueiredo <[hidden email]> escreveu:
The page https://stackoverflow.com/questions/13517526/difference-between-gcc-d-fortify-source-1-and-d-fortify-source-2
says:

"If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1
(gcc -O1) and above, checks that shouldn't change the behavior of
conforming programs are performed. With _FORTIFY_SOURCE set to 2, some
more checking is added, but some conforming programs might fail."

"some conforming programs might fail" caught my eye...
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Roberto Ierusalimschy
> I've used gcc 9.3.0 to compile the same sources with the same flags and the
> code runs fine with any cflags combination, so I am sure it's something
> related to GCC 10.1.0.

We got a report about this problem some weeks ago. See

  http://lua-users.org/lists/lua-l/2020-07/msg00001.html

-- Roberto
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Andrew Gierth
In reply to this post by Eduardo Barthel
>>>>> "Eduardo" == Eduardo Bart <[hidden email]> writes:

 Eduardo> I have a large lua application that works fine with Lua 5.3
 Eduardo> and Lua 5.4 that is shipped by my Linux package manager.
 Eduardo> However I've downloaded the Lua 5.4 sources and compiled
 Eduardo> myself and when using it I got a crash.

Known, reported and fixed bug in GCC 10:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

Workaround is -fno-ipa-sra

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
In reply to this post by Roberto Ierusalimschy
could this be related to
/* maximum length of the conversion of a number to a string */
#define MAXNUMBER2STR   50
where the string is allocated on the stack with an array of bytes whose size (including the null terminator) is not a multiple of the word size? Causing some internal bug in the stack slots allocator in GCC 10.1?
Note that "void luaO_tostring" is the only function where it is allocated this way. This may cause issue when this function is inlined (probably alignment problems).

May be this is solved by just making this a multiple of 8 bytes (64-bit architectures) or 16 bytes (128-bit architectures).
However how can even on a 64-bit architecture this generate a numeric string that could be 49 bytes long plus a null ?
May be the type for number can have its bitsize asserted to define the length that is needed for the mantissa, the exponent, the signs and the dot. If this is too complicate, why not just aligning 50 to the next multiple of 8 or 16, i.e. setting it to 56 or 64?
#define MAXNUMBER2STR   56
This will be more than what is necessary, but at least it could avoid the alignment problem when inlining.

For numbers defined as IEEE 64-bit doubles, the decimal expansion can at msot produce a string which has 15 to 17 decimal digits, a signs, a dot, an "E" separator, and up to 3 digits plus a sign for the exponent, so the string has at most 24 characters plus a nul.

Only if characters are represented using double-byte digits or exponents (when not using the "C" locale in Basic Latin) the string soace could be larger than 25 bytes. 50 was defined as if all characters could be double-byte.

luaconf.h also contains various macros testing if numbers are complied with long doubles and sets the default prefcision to 19 digits of mantissa (14 digits for doubles, 7 digits for floats). The conversion is made using    l_sprintf():

/* @@ l_sprintf is equivalent to 'snprintf' or 'sprintf' in C89.
** (All uses in Lua have only one format item.)
*/
#if !defined(LUA_USE_C89)
#define l_sprintf(s,sz,f,i)     snprintf(s,sz,f,i)
#else
#define l_sprintf(s,sz,f,i)     ((void)(sz), sprintf(s,f,i))
#endif
This macro does not use the (sz) parameter, if it uses sprintf(). But in C89 it uses snprintf(), and in GCC, snprintf may be an intrinsic which may be inlined by assembly instructions by the compiler depending on the size given (when using the advanced optimization), instead of performing a function call to the C library: this is where an alignment problem may possibly occur, where stack slots position may be incorrectly computed. One way to workaround this bug could simply as well be to round the size to a multiple of 8 or 16.

Here the crash exposes the use of snprintf() as a function call; may be this function has some acceleration tweaks for the optimized version, such as manipulating the stack and using vectored instructions; but vector instructions require some registers to be possibly saved to the stack: if stack slot positions are incorrectly computed, what is saved before using vector instructions may be overwritten by the vector instruction ins the same stack. Aligning the size to a correct multiple may help prevent this bug.

But I note an other warning in luaconf.h (this time for a float rather than a double or long double):
/* @@ lua_numbertointeger converts a float number with an integral value
** to an integer, or returns 0 if float is not within the range of
** a lua_Integer.  (The range comparisons are tricky because of
** rounding. The tests here assume a two-complement representation,
** where MININTEGER always has an exact representation as a float;
** MAXINTEGER may not have one, and therefore its conversion to float
** may have an ill-defined value.)
*/
#define lua_numbertointeger(n,p) \
  ((n) >= (LUA_NUMBER)(LUA_MININTEGER) && \
   (n) < -(LUA_NUMBER)(LUA_MININTEGER) && \
      (*(p) = (LUA_INTEGER)(n), 1))

(Here the problem is with MAXINTEGER for converting integers.)



Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Andrew Gierth
>>>>> "Philippe" == Philippe Verdy <[hidden email]> writes:

 Philippe> could this be related to
 Philippe> /* maximum length of the conversion of a number to a string */
 Philippe> #define MAXNUMBER2STR   50

No.

It's to do with the fact that the lua value is a tagged union which can
be either a 64-bit integer or 64-bit double. Optimization code which
is taking advantage of the fact that the type is known at the callsite
to produce specialized variants of the called function was confusing the
types and passing them in incorrect registers as a result.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
I see... Thanks. Anyway I don't know why you use this magic value 50 which is twice more than what is needed, and not a multiple of 8 or 16.
Even with long doubles, a buffer size of 32 bytes would be enough (we're not converting to UTF-8, just to plain ASCII, with vs[n]printf() I think (I may be wrong with some double-byte locales)
But if you ever think about the possibility of double byte default locales in C, may be 50 is not enough and 64 would then be safer.

This part of "luaconf.h" is a bit tricky, seems to have been tweaked/adjusted manually with various test/fail/retries. There's no real test if this is safe when porting. We just seem to assume that only common IEEE 754 sizes will be used (including 80-bit long doubles, but why not 128-bit on some archs? What is there's a new architecture supporting 256-bit "long doubles" larger than the ISS 754 minimums just defined for "float" and "double"?)
There's no compiler directives to assert the implicit size constraints and still this allocates more than what is needed in common x64 and ARM64 archs used today (even the next coming Mac will use ARM64 after the 68k, PPC and x64 adventures, but here I think about architectures that would want to support Lua on large application servers with more "exotic" processsors and high levels of parallelism; including new processor types for IA like what Google is developing and now selling, or what is used in GPUs with their dedicated recompilers and native APIs; nVidia in particular hides lots of details).


Le mar. 18 août 2020 à 23:43, Andrew Gierth <[hidden email]> a écrit :
>>>>> "Philippe" == Philippe Verdy <[hidden email]> writes:

 Philippe> could this be related to
 Philippe> /* maximum length of the conversion of a number to a string */
 Philippe> #define MAXNUMBER2STR   50

No.

It's to do with the fact that the lua value is a tagged union which can
be either a 64-bit integer or 64-bit double. Optimization code which
is taking advantage of the fact that the type is known at the callsite
to produce specialized variants of the called function was confusing the
types and passing them in incorrect registers as a result.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
Note that octuple precision (256-bits) is already part of the IEEE 754 as an approved extension since long (already used in 2006 on Apple G4).
Apple may be tempted to renew the support for it in its next ARM-based processor...

Le mar. 18 août 2020 à 23:58, Philippe Verdy <[hidden email]> a écrit :
I see... Thanks. Anyway I don't know why you use this magic value 50 which is twice more than what is needed, and not a multiple of 8 or 16.
Even with long doubles, a buffer size of 32 bytes would be enough (we're not converting to UTF-8, just to plain ASCII, with vs[n]printf() I think (I may be wrong with some double-byte locales)
But if you ever think about the possibility of double byte default locales in C, may be 50 is not enough and 64 would then be safer.

This part of "luaconf.h" is a bit tricky, seems to have been tweaked/adjusted manually with various test/fail/retries. There's no real test if this is safe when porting. We just seem to assume that only common IEEE 754 sizes will be used (including 80-bit long doubles, but why not 128-bit on some archs? What is there's a new architecture supporting 256-bit "long doubles" larger than the ISS 754 minimums just defined for "float" and "double"?)
There's no compiler directives to assert the implicit size constraints and still this allocates more than what is needed in common x64 and ARM64 archs used today (even the next coming Mac will use ARM64 after the 68k, PPC and x64 adventures, but here I think about architectures that would want to support Lua on large application servers with more "exotic" processsors and high levels of parallelism; including new processor types for IA like what Google is developing and now selling, or what is used in GPUs with their dedicated recompilers and native APIs; nVidia in particular hides lots of details).


Le mar. 18 août 2020 à 23:43, Andrew Gierth <[hidden email]> a écrit :
>>>>> "Philippe" == Philippe Verdy <[hidden email]> writes:

 Philippe> could this be related to
 Philippe> /* maximum length of the conversion of a number to a string */
 Philippe> #define MAXNUMBER2STR   50

No.

It's to do with the fact that the lua value is a tagged union which can
be either a 64-bit integer or 64-bit double. Optimization code which
is taking advantage of the fact that the type is known at the callsite
to produce specialized variants of the called function was confusing the
types and passing them in incorrect registers as a result.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
Also various IBM mainframes all have quadruple-precision (128-bit) "long doubles", with hardware implementation in System z10 since 2008.

Le mer. 19 août 2020 à 00:02, Philippe Verdy <[hidden email]> a écrit :
Note that octuple precision (256-bits) is already part of the IEEE 754 as an approved extension since long (already used in 2006 on Apple G4).
Apple may be tempted to renew the support for it in its next ARM-based processor...

Le mar. 18 août 2020 à 23:58, Philippe Verdy <[hidden email]> a écrit :
I see... Thanks. Anyway I don't know why you use this magic value 50 which is twice more than what is needed, and not a multiple of 8 or 16.
Even with long doubles, a buffer size of 32 bytes would be enough (we're not converting to UTF-8, just to plain ASCII, with vs[n]printf() I think (I may be wrong with some double-byte locales)
But if you ever think about the possibility of double byte default locales in C, may be 50 is not enough and 64 would then be safer.

This part of "luaconf.h" is a bit tricky, seems to have been tweaked/adjusted manually with various test/fail/retries. There's no real test if this is safe when porting. We just seem to assume that only common IEEE 754 sizes will be used (including 80-bit long doubles, but why not 128-bit on some archs? What is there's a new architecture supporting 256-bit "long doubles" larger than the ISS 754 minimums just defined for "float" and "double"?)
There's no compiler directives to assert the implicit size constraints and still this allocates more than what is needed in common x64 and ARM64 archs used today (even the next coming Mac will use ARM64 after the 68k, PPC and x64 adventures, but here I think about architectures that would want to support Lua on large application servers with more "exotic" processsors and high levels of parallelism; including new processor types for IA like what Google is developing and now selling, or what is used in GPUs with their dedicated recompilers and native APIs; nVidia in particular hides lots of details).


Le mar. 18 août 2020 à 23:43, Andrew Gierth <[hidden email]> a écrit :
>>>>> "Philippe" == Philippe Verdy <[hidden email]> writes:

 Philippe> could this be related to
 Philippe> /* maximum length of the conversion of a number to a string */
 Philippe> #define MAXNUMBER2STR   50

No.

It's to do with the fact that the lua value is a tagged union which can
be either a 64-bit integer or 64-bit double. Optimization code which
is taking advantage of the fact that the type is known at the callsite
to produce specialized variants of the called function was confusing the
types and passing them in incorrect registers as a result.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Ranier Vilela-2
In reply to this post by Philippe Verdy-2


Em ter., 18 de ago. de 2020 às 18:38, Philippe Verdy <[hidden email]> escreveu:
could this be related to
/* maximum length of the conversion of a number to a string */
#define MAXNUMBER2STR   50
where the string is allocated on the stack with an array of bytes whose size (including the null terminator) is not a multiple of the word size? Causing some internal bug in the stack slots allocator in GCC 10.1?
Note that "void luaO_tostring" is the only function where it is allocated this way. This may cause issue when this function is inlined (probably alignment problems).

May be this is solved by just making this a multiple of 8 bytes (64-bit architectures) or 16 bytes (128-bit architectures).
However how can even on a 64-bit architecture this generate a numeric string that could be 49 bytes long plus a null ?
May be the type for number can have its bitsize asserted to define the length that is needed for the mantissa, the exponent, the signs and the dot. If this is too complicate, why not just aligning 50 to the next multiple of 8 or 16, i.e. setting it to 56 or 64?
#define MAXNUMBER2STR   56
I agree. Could be 64.
This can avoid mistakes with address calcs, mainly with aggressive optimizations.
Everyone is power of two, it makes no sense to use decimal values, which are for humans, not machines.

regards,
Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
In reply to this post by Philippe Verdy-2
As well 128-bit quadruple precision is now required for some industries, notably for high-precision geography (OASIS standards), and satellite positioning.

Le mer. 19 août 2020 à 00:05, Philippe Verdy <[hidden email]> a écrit :
Also various IBM mainframes all have quadruple-precision (128-bit) "long doubles", with hardware implementation in System z10 since 2008.

Le mer. 19 août 2020 à 00:02, Philippe Verdy <[hidden email]> a écrit :
Note that octuple precision (256-bits) is already part of the IEEE 754 as an approved extension since long (already used in 2006 on Apple G4).
Apple may be tempted to renew the support for it in its next ARM-based processor...

Le mar. 18 août 2020 à 23:58, Philippe Verdy <[hidden email]> a écrit :
I see... Thanks. Anyway I don't know why you use this magic value 50 which is twice more than what is needed, and not a multiple of 8 or 16.
Even with long doubles, a buffer size of 32 bytes would be enough (we're not converting to UTF-8, just to plain ASCII, with vs[n]printf() I think (I may be wrong with some double-byte locales)
But if you ever think about the possibility of double byte default locales in C, may be 50 is not enough and 64 would then be safer.

This part of "luaconf.h" is a bit tricky, seems to have been tweaked/adjusted manually with various test/fail/retries. There's no real test if this is safe when porting. We just seem to assume that only common IEEE 754 sizes will be used (including 80-bit long doubles, but why not 128-bit on some archs? What is there's a new architecture supporting 256-bit "long doubles" larger than the ISS 754 minimums just defined for "float" and "double"?)
There's no compiler directives to assert the implicit size constraints and still this allocates more than what is needed in common x64 and ARM64 archs used today (even the next coming Mac will use ARM64 after the 68k, PPC and x64 adventures, but here I think about architectures that would want to support Lua on large application servers with more "exotic" processsors and high levels of parallelism; including new processor types for IA like what Google is developing and now selling, or what is used in GPUs with their dedicated recompilers and native APIs; nVidia in particular hides lots of details).


Le mar. 18 août 2020 à 23:43, Andrew Gierth <[hidden email]> a écrit :
>>>>> "Philippe" == Philippe Verdy <[hidden email]> writes:

 Philippe> could this be related to
 Philippe> /* maximum length of the conversion of a number to a string */
 Philippe> #define MAXNUMBER2STR   50

No.

It's to do with the fact that the lua value is a tagged union which can
be either a 64-bit integer or 64-bit double. Optimization code which
is taking advantage of the fact that the type is known at the callsite
to produce specialized variants of the called function was confusing the
types and passing them in incorrect registers as a result.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96040

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
In reply to this post by Ranier Vilela-2
The default 50 would be fine only on 16-bit machines, which are out of use since long. But even in that case, those machines had more limited precisions for the floating points, and number would be complied most probably only as a 32-bit float (still requiring often software emulation), so the maximum string length 50 is also overkill. Only on those machines you would want to save some bytes on the stack, so you would reduce 50 to just 32 and it would still mich be large enough for 32-bit floats; may be "long double" could be supported by (slow) libraries but the compiler would just compute at most 64-bits like for "double", four which a 32-byte buffer (including the null terminator) is still enough. As these specialized types of small machines have lot of other limitations, "luaconf.h" would need to be tweaked specially to compile Lua on them (e.g. for small microcontrollers driving simple hardware bots or tools not requiring high-speed control).
50 bytes is wrong for all cases. 48 bytes would be better and would still work (the extra 16 bytes on the stack are for the one or two parameters of the function call and possibly one for the returning point or a save slot ion the stack for a register that will be used by inlined functions). Still there won't be any alignment problem with today's 32-bit or 64-bit machines, or newer 128-bit machines (or existing 64-bit machines like x64 or ARM64 with 128-bit vector extension that could have hardware support integrated in their ISA).
The value "50" must just have been a rough estimation for humans and makes no sense, it just adds extra complications for the compilers and possible causes of bugs (or could prevent some optimizations to occur). Let's remember that Lu very frequently performs conversions between numbers and strings: this should be efficient without extra complication, and all optimizer hints should be usable (notably compilers could use inlinable intrinsics with more aggressive register allocation and serialized instructions or vector instructions, rather than using the stack and function calls if it can. This can boost a Lua program quite a lot in frequent hotspots: these number<->string conversions are such hotspots in Lua.


Le mer. 19 août 2020 à 00:07, Ranier Vilela <[hidden email]> a écrit :


Em ter., 18 de ago. de 2020 às 18:38, Philippe Verdy <[hidden email]> escreveu:
could this be related to
/* maximum length of the conversion of a number to a string */
#define MAXNUMBER2STR   50
where the string is allocated on the stack with an array of bytes whose size (including the null terminator) is not a multiple of the word size? Causing some internal bug in the stack slots allocator in GCC 10.1?
Note that "void luaO_tostring" is the only function where it is allocated this way. This may cause issue when this function is inlined (probably alignment problems).

May be this is solved by just making this a multiple of 8 bytes (64-bit architectures) or 16 bytes (128-bit architectures).
However how can even on a 64-bit architecture this generate a numeric string that could be 49 bytes long plus a null ?
May be the type for number can have its bitsize asserted to define the length that is needed for the mantissa, the exponent, the signs and the dot. If this is too complicate, why not just aligning 50 to the next multiple of 8 or 16, i.e. setting it to 56 or 64?
#define MAXNUMBER2STR   56
I agree. Could be 64.
This can avoid mistakes with address calcs, mainly with aggressive optimizations.
Everyone is power of two, it makes no sense to use decimal values, which are for humans, not machines.

regards,
Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
Note that for octuple precision (256-bit) floating-point in IEEE754, the minimum buffer size is 82 bytes (including the null terminator, but excluding any optional group separator), so with alignment set to 8-bytes (64-bit architecture) or 16-bytes (128-bit arch), the suitable buffer size in the stack would be 96 bytes to display the full precision (you could still reduce it to 64 bytes if you accept to not display the full precision that would typically be used only for intermediate calculations of aggregates on large datasets with high precision, such as collecting many measurements: this is still enough for nuclear research today given the existing precision of units, today around 20 decimal digits for some physical scales, this may evolve soon with the reform of the meter in SI; the high precision is just needed to collect many quantic measurements at very high frequencies before processing them and rounding them because they have large margins of randomness where the wanted precision is hidden with lot of quantic noises).
256-bit floating point is already interesting searchers on IA and for processing "big data" sets that are for now still seen as very chaotic (e.g. in automated financial/trading applications, or meteorological or simulations in fluid mechanic, or in massively parallelized applications with lots of users like multiplayer online games on commercial game servers, where many users could feed their own scripts: many concurrent small Lua scripts, changeable/loadable in real-time without stopping the server)

Quad precision (128-bit) is already used in high-precision 3D manufacturing to control bots. I think they are already used in radioastronomy for controlling the shape of mirrors, or in the nuclear research industry in accelerators or for simulations of for researches on black matter and black energy (I've read an article suggesting its use for large arrays of telescopes); it may already have application in cryptography to speed up and secure the generation of keys with more challenging algorithms. Given it is already available on consumer markets since several decennials and there's an incentive with 3D rendering of light effects and raytracing in popular games, the existing 64-bit architecture will support it as part of their native vector instruction extensions.
Cloud computing with giant servers may already use them but would benefit as well from hardware implementations instead of relying on slow emulation (and energy-inefficient) with software libraries.

If you don't want to extend the stack size (and still don't plan to support octuple precision with their full precision, except by tweaking luaconf.h for these corner-side experimental architectures), using 48 instead of 50 would just be fine.


Le mer. 19 août 2020 à 00:50, Philippe Verdy <[hidden email]> a écrit :
The default 50 would be fine only on 16-bit machines, which are out of use since long. But even in that case, those machines had more limited precisions for the floating points, and number would be complied most probably only as a 32-bit float (still requiring often software emulation), so the maximum string length 50 is also overkill. Only on those machines you would want to save some bytes on the stack, so you would reduce 50 to just 32 and it would still mich be large enough for 32-bit floats; may be "long double" could be supported by (slow) libraries but the compiler would just compute at most 64-bits like for "double", four which a 32-byte buffer (including the null terminator) is still enough. As these specialized types of small machines have lot of other limitations, "luaconf.h" would need to be tweaked specially to compile Lua on them (e.g. for small microcontrollers driving simple hardware bots or tools not requiring high-speed control).
50 bytes is wrong for all cases. 48 bytes would be better and would still work (the extra 16 bytes on the stack are for the one or two parameters of the function call and possibly one for the returning point or a save slot ion the stack for a register that will be used by inlined functions). Still there won't be any alignment problem with today's 32-bit or 64-bit machines, or newer 128-bit machines (or existing 64-bit machines like x64 or ARM64 with 128-bit vector extension that could have hardware support integrated in their ISA).
The value "50" must just have been a rough estimation for humans and makes no sense, it just adds extra complications for the compilers and possible causes of bugs (or could prevent some optimizations to occur). Let's remember that Lu very frequently performs conversions between numbers and strings: this should be efficient without extra complication, and all optimizer hints should be usable (notably compilers could use inlinable intrinsics with more aggressive register allocation and serialized instructions or vector instructions, rather than using the stack and function calls if it can. This can boost a Lua program quite a lot in frequent hotspots: these number<->string conversions are such hotspots in Lua.


Le mer. 19 août 2020 à 00:07, Ranier Vilela <[hidden email]> a écrit :


Em ter., 18 de ago. de 2020 às 18:38, Philippe Verdy <[hidden email]> escreveu:
could this be related to
/* maximum length of the conversion of a number to a string */
#define MAXNUMBER2STR   50
where the string is allocated on the stack with an array of bytes whose size (including the null terminator) is not a multiple of the word size? Causing some internal bug in the stack slots allocator in GCC 10.1?
Note that "void luaO_tostring" is the only function where it is allocated this way. This may cause issue when this function is inlined (probably alignment problems).

May be this is solved by just making this a multiple of 8 bytes (64-bit architectures) or 16 bytes (128-bit architectures).
However how can even on a 64-bit architecture this generate a numeric string that could be 49 bytes long plus a null ?
May be the type for number can have its bitsize asserted to define the length that is needed for the mantissa, the exponent, the signs and the dot. If this is too complicate, why not just aligning 50 to the next multiple of 8 or 16, i.e. setting it to 56 or 64?
#define MAXNUMBER2STR   56
I agree. Could be 64.
This can avoid mistakes with address calcs, mainly with aggressive optimizations.
Everyone is power of two, it makes no sense to use decimal values, which are for humans, not machines.

regards,
Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Sam Trenholme
In reply to this post by Philippe Verdy-2
> 128-bit quadruple precision

The thing about IEEE 128-bit quadruple precision floats is that they
only have hardware support in the POWER9, the IBM S/390 from the 1990s,
and z/Architecture systems.  We’re talking systems which cost at least
$3000 and go up very quickly from there.

It *is* possible to change the default number type in luaconf.h (the
#define LUA_NUMBER) to something else, such as IEEE binary128 or
decimal128 float, as long as one’s C compiler supports the type.  Of
course, if using a type like that, it’s probably a good idea to change
LUA_NUMBER_FMT to hold more digits (and make sure the buffers can hold
the digits).

As it turns out, with mainstream processors, instead of having more
widespread support for 80-bit floats, ARM processors -- read, most of
the real-world processors out there in our smartphone-addicted age --
are instead increasing support for 16-bit float types.  Armv8.1-M and
ARMv8.2-A added 16-bit floats (1 sign bit, 5 exponent bits, 10 mantissa
bits), ARMv8.6-A added support for another 16-bit float format,
“BFloat16” (1 sign bit, 8 exponent bits, 7 mantissa bits).

Of course, someone *could* extend Lua to use GNU Multiple Precision
Arithmetic Library or what not as the native number type, but Lua is not
Python, and doing so would make Lua a lot larger and a lot slower.  
64-bit floats, the default, is usually good enough, keeps Lua small (I
like having a full Lua-5.1 derived interpreter which fits in 118,784
bytes) and when it’s not, there are solutions like
https://github.com/LuaDist/mapm out there.
Reply | Threaded
Open this post in threaded view
|

Re: Crash in tostringbuff when -D_FORTIFY_SOURCE=2 is not defined in Lua 5.4

Philippe Verdy-2
I'm talking about the newcoming processors that integrate more and more coprocessor features and vector instructions as extensions (grouping ranges of registers by pairs to create larger registers, and with dedicated ALU/FPU units that are now increasingly parallized with multiple processing channels. There's a trend now, with AI units, GPUs, and as well a need for new kinds of applications that will process huge amounts of data containing tiny bits of information that can only be detected within a large amount of noise, currently hidden/masked by the limited precision.
Apple, Google are already selling their new processors; even if they are based on a 64-bit architecture, they contain the vector extensions and 128 bit extensions  We'll see more and more use of these types because this is not for common desktop or web applications we used until now (including web apps for mobiles), as they are massively interconnected over a faster and larger network. And the number of devices with processing capabilities explodes; The industry finds new applications everyday: we are no longer at the time of experiments of specialized applications for specific domains by specific installations. We find them now everywhere in all sorts of objects (IOTs are now there for long, may be their local processing is limited but they work within a very capable and very wide network that provides and largely extends their services and usability by more nad more users). Even a basic users could still use them with small basic Lua scripts that will integrate into the compound system and will want to use the same datatype (it will not necessarily be slow)


Le mer. 19 août 2020 à 07:03, Sam Trenholme <[hidden email]> a écrit :
> 128-bit quadruple precision

The thing about IEEE 128-bit quadruple precision floats is that they
only have hardware support in the POWER9, the IBM S/390 from the 1990s,
and z/Architecture systems.  We’re talking systems which cost at least
$3000 and go up very quickly from there.

It *is* possible to change the default number type in luaconf.h (the
#define LUA_NUMBER) to something else, such as IEEE binary128 or
decimal128 float, as long as one’s C compiler supports the type.  Of
course, if using a type like that, it’s probably a good idea to change
LUA_NUMBER_FMT to hold more digits (and make sure the buffers can hold
the digits).

As it turns out, with mainstream processors, instead of having more
widespread support for 80-bit floats, ARM processors -- read, most of
the real-world processors out there in our smartphone-addicted age --
are instead increasing support for 16-bit float types.  Armv8.1-M and
ARMv8.2-A added 16-bit floats (1 sign bit, 5 exponent bits, 10 mantissa
bits), ARMv8.6-A added support for another 16-bit float format,
“BFloat16” (1 sign bit, 8 exponent bits, 7 mantissa bits).

Of course, someone *could* extend Lua to use GNU Multiple Precision
Arithmetic Library or what not as the native number type, but Lua is not
Python, and doing so would make Lua a lot larger and a lot slower. 
64-bit floats, the default, is usually good enough, keeps Lua small (I
like having a full Lua-5.1 derived interpreter which fits in 118,784
bytes) and when it’s not, there are solutions like
https://github.com/LuaDist/mapm out there.