Invalid memcpy call

[jakub-zwolakowski]

Analysis of JerryScript test test-literal-storage.c using TrustInSoft CI has shown that the function lit_is_utf8_string_magic (in file lit-magic-strings.c) sometimes calls memcpy with a null pointer as argument:
https://github.com/jakub-zwolakowski/jerryscript/blob/90c97fee6ca924a7c6bf42e07705cbe720ac89c0/jerry-core/lit/lit-magic-strings.c#L218

Passing a null pointer to a C library function like memcpy is dangerous, even if done together with the size argument equal zero.

Some compilers perform optimizations based on the assumption that such a function never receives a null pointer. GCC started to optimize this behavior aggressively in its 4.9 version with the optimization -fdelete-null-pointer-checks (it is discussed in this blog post: http://blog.mycre.ws/articles/bind-and-gcc-49/). This is an example of a program that passes a null pointer to memcpy:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

__attribute__((noinline))
void f(char *p, int n) {
  if (n > 10) abort();
  memcpy(p, "0123456789", n);
  if (p == NULL) {
    printf("f was called with NULL\n");
  }
  else {
    printf("f was called with a non-NULL pointer\n");
    printf("this is the value of p:%p or 0x%llx\n", p, (unsigned long long) p);
  }
}

int main(void) {
  f(NULL, 0);
}

Binaries produced when compiling this program with GCC 7.5.0 print different results when run with different optimization settings:

$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc test.c -O0 && ./a.out 
f was called with NULL
$ gcc test.c -O2 && ./a.out 
f was called with a non-NULL pointer
this is the value of p:(nil) or 0x0

(You can see how the generated code looks like in Compiler Explorer for these two cases in GCC 10.2: with optimization level 0 and with optimization level 2. Note that in the latter case the whole p == NULL check disappears.)

This is not a GCC bug. This behavior is present in GCC already for several versions. What the compiler does is legal as the program passes a null pointer to memcpy which is forbidden by clause 7.1.4:1 in C17 (and this clause is already present in C99):

Each of the following statements applies unless explicitly stated otherwise in the detailed descrip-
tions that follow: If an argument to a function has an invalid value (such as a value outside the
domain of the function, or a pointer outside the address space of the program, or a null pointer, or a
pointer to non-modifiable storage when the corresponding parameter is not const-qualified) or a
type (after promotion) not expected by a function with variable number of arguments, the behavior
is undefined.

The program's behavior is undefined, so GCC can rightfully do whatever it finds most appropriate. And what it finds most appropriate (for optimization reasons) in this case is to suppose that any pointer p that was passed as an argument to memcpy is non-null.

[ossy-szeged]

Thanks for the report. Could you please provide an example triggers this issue?

[jakub-zwolakowski]

The example which triggers this issue is tests/unit-core/test-literal-storage.c

TrustInSoft CI is a C source code interpreter which follows the execution path of the program and detects all the Undefined Behaviors which are on this path. In this case, the entry point of this execution path is the function main defined in test-literal-storage.c.

You can see the detected issue here, in test number 29: https://ci.trust-in-soft.com/projects/jakub-zwolakowski/jerryscript/11?page=2&test=29

The Explore button allows you to understand the cause of the Undefined Behavior.

When I go up the callstack, I see that the NULL value comes from this call in the main function in test-literal-storage.c:

/* Add empty string. */
ecma_find_or_create_literal_string (NULL, 0);

[rerobika]

Hi @jakub-zwolakowski!

Thanks for the analysis, here comes mine:

That's true that at this point the memcmp 2nd argument is NULL, however the string_size is ensured to be 0. There is an assertion above: JERRY_ASSERT (string_p != NULL || string_size == 0);

Passing a null pointer to a C library function like memcpy is dangerous, even if done together with the size argument equal zero.

• The "dangerous" adjective sounds a bit unprofessional for me since this behavior is documented by the standard.

According to C99 standard (7.21.1/2):

Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters.

So passing NULL when the size is 0 for memcmp is completely valid.

[pascal-cuoq]

Hello, let me emphasize the important words here:

C99 7.1.4:1 (https://port70.net/~nsz/c/c99/n1256.html#7.1.4p1 )

Each of the following statements applies unless explicitly stated otherwise in the detailed descriptions that follow: If an argument to a function has an invalid value (such as a value outside the domain of the function, or a pointer outside the address space of the program, or a null pointer, or a pointer to non-modifiable storage when the corresponding parameter is not const-qualified) or a type (after promotion) not expected by a function with variable number of arguments, the behavior is undefined

C99 7.21.1:2 (https://port70.net/~nsz/c/c99/n1256.html#7.21.1p2 )

Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters.

Please humor me for one minute and predict the output of this program:

#include <string.h>
#include <stdio.h>

__attribute__((noinline)) // assume function is in different compilation unit
int f(char *p, char *q, int n) {
    static int x;
    x = memcmp(p, q, n);
    return (p == NULL);
}

int main(void) {
    char c = 'a';
    char *q = &c;
    char *p = NULL;
    printf("p is NULL: %d\n", f(p, q, 0));
}

The program above, when compiled with gcc -O2, prints p is NULL: 0. Please see for yourself with this Compiler Explorer link: https://gcc.godbolt.org/z/WWTbb6 or run it with any GCC released less than 4 years ago. You can even get rid of the __attribute__((noinline)), if you think that this is where the “trick” lies, by putting the f function in a separate compilation unit than the main function.

The reason GCC is allowed to compile the program above to a binary that prints p is NULL: 0 is that the program exhibits undefined behavior by passing NULL to memcmp, therefore any behavior is correct. If you want GCC to be allowed to generate a binary in which anything goes and any behavior is correct for Jerryscript, you only need to pass NULL to memcmp in Jerryscript.

Further reading: https://blog.mycre.ws/articles/bind-and-gcc-49/

[akosthekiss]

• Unfortunately, the standard indeed specifies calling memcmp or memcpy with a null pointer as undefined behaviour.
• Is it memcpy or memcmp? The original issue report (title & first comment) is about memcpy, but further discussion is about memcmp.
• A more important question is whether the UB can ever be hit in real code? I.e., is it the engine or the test case that needs a fix? I cannot find ecma_find_or_create_literal_string (NULL, 0); anywhere but in the test case. Can a null pointer reach lit_is_utf8_string_magic on any other path (either from within the engine or via an API call)?

[jakub-zwolakowski]

• Is it memcpy or memcmp? The original issue report (title & first comment) is about memcpy, but further discussion is about memcmp.

It should be memcmp right from the beginning. Sorry, this is my mistake, I've mixed up the two at some point... Fortunately, in this context the difference is irrelevant as both are affected in the same way by these C99 clauses.

• A more important question is whether the UB can ever be hit in real code? I.e., is it the engine or the test case that needs a fix? I cannot find ecma_find_or_create_literal_string (NULL, 0); anywhere but in the test case. Can a null pointer reach lit_is_utf8_string_magic on any other path (either from within the engine or via an API call)?

The perimeter of analysis which I've performed was defined by the test suite. In order to answer your question I would have to conduct an exhaustive analysis and prove absence of any path in the program that could lead to this UB.

So for now what I'm saying is simply that I don't know if this UB can be hit in the real code. Maybe it can maybe it can't, though I have no proof that it can't.

If I were to guess, I'd say that it is a possibility. Looking at the callstack in this particular case:
ecma_find_special_string@1←ecma_new_ecma_string_from_utf8@1←ecma_find_or_create_literal_string@1←main@52
If any of these 3 functions is ever called with arguments (NULL, 0) then the UB will be hit. And the assertions (mentioned above) do not prohibit it.