Fix a few issues which can lead to undefined-behaviour #1730
Conversation
JerryScript-DCO-1.0-Signed-off-by: Zsolt Borbély [email protected]
| @@ -126,7 +126,8 @@ re_bytecode_list_insert (re_bytecode_ctx_t *bc_ctx_p, /**< RegExp bytecode conte | |||
| inline ecma_char_t __attr_always_inline___ | |||
| re_get_char (uint8_t **bc_p) /**< pointer to bytecode start */ | |||
| { | |||
| ecma_char_t chr = *((ecma_char_t *) *bc_p); | |||
| ecma_char_t chr; | |||
| memcpy (&chr, *bc_p, sizeof (ecma_char_t)); | |||
There was a problem hiding this comment.
What exactly was undefined behaviour in the original code? Is memcpy the only way to get around it? As it is a call to a libc function, it is much more expensive than a simple cast.
There was a problem hiding this comment.
Reading uint16 from byte aligned address is undefined in C.
There was a problem hiding this comment.
Ah, that. But cannot we have a macro that does the uint16 read from a byte pointer "in place" without the function call overhead (and the loop overhead of memcpy)?
There was a problem hiding this comment.
Any answer to this, even though it got merged already?...
There was a problem hiding this comment.
As far as I know most compilers are proud to optimize this.
There was a problem hiding this comment.
If we don't trust them we could create a macro. Problem is, if the architecture supports unaligned read, we are expecting the compiler to deoptimize our macro.
There was a problem hiding this comment.
Btw GCC has a flag to not optimize this: -fno-builtin-memcpy
There was a problem hiding this comment.
The thing is that we have our own memcpy implementation in jerry-libc and I'm not sure that gcc can optimize it in that case. Especially as we are compiling jerry with -fno-builtin, which has effect on memcpy as well.
JerryScript-DCO-1.0-Signed-off-by: Zsolt Borbély [email protected]