Skip to content

ext/reflection causes stack overflow under valgrind when CC=clang #11713

Closed
Closed
ext/reflection causes stack overflow under valgrind when CC=clang#11713
Labels
BugCategory: EngineStatus: Verified

Description

@ju1ius
ju1ius
opened on Jul 16, 2023

Description

Please refer to this reproducer repository.

Using php compiled with CC=clang and --enable-debug --with-valgrind, instanciating any Reflection object causes valgrind to crash.

Here's an exerpt of the gdb backtrace:

... the rest is just an infinite loop
0x0000000000a00f01 in object_init_ex (arg=0x1ffeffd5b8, class_type=0x4c96f20) at Zend/zend_API.c:1794
0x0000000000b06622 in zend_throw_exception_zstr (exception_ce=0x4c96f20, message=0x4dec7d0, code=0) at Zend/zend_exceptions.c:828
0x0000000000b02b06 in zend_throw_exception (exception_ce=0x4c96f20, message=0x4dec6b0 "Cannot instantiate abstract class Error", code=0) at Zend/zend_exceptions.c:848
0x00000000009f86e6 in zend_throw_error (exception_ce=0x4c96f20, format=0xd569e6 "Cannot instantiate abstract class %s") at Zend/zend.c:1714
0x0000000000a00d6c in _object_and_properties_init (arg=0x1ffeffd888, class_type=0x4c96f20, properties=0x0) at Zend/zend_API.c:1755
0x0000000000a00f01 in object_init_ex (arg=0x1ffeffd5b8, class_type=0x4c96f20) at Zend/zend_API.c:1794
0x0000000000b06622 in zend_throw_exception_zstr (exception_ce=0x4c96f20, message=0x4dec7d0, code=0) at Zend/zend_exceptions.c:828
0x0000000000b02b06 in zend_throw_exception (exception_ce=0x4c96f20, message=0x4dec6b0 "Cannot instantiate abstract class Error", code=0) at Zend/zend_exceptions.c:848
0x00000000009f86e6 in zend_throw_error (exception_ce=0x4c96f20, format=0xd569e6 "Cannot instantiate abstract class %s") at Zend/zend.c:1714
0x0000000000a00d6c in _object_and_properties_init (arg=0x1ffeffd888, class_type=0x4c96f20, properties=0x0) at Zend/zend_API.c:1755
0x0000000000a00f01 in object_init_ex (arg=0x1ffeffd888, class_type=0x4c96f20) at Zend/zend_API.c:1794
0x0000000000b06622 in zend_throw_exception_zstr (exception_ce=0x4c96f20, message=0x4dec630, code=0) at Zend/zend_exceptions.c:828
0x0000000000b02b06 in zend_throw_exception (exception_ce=0x4c96f20, message=0x4dec510 "Cannot instantiate abstract class Error", code=0) at Zend/zend_exceptions.c:848
0x00000000009f86e6 in zend_throw_error (exception_ce=0x4c96f20, format=0xd569e6 "Cannot instantiate abstract class %s") at Zend/zend.c:1714
0x0000000000a00d6c in _object_and_properties_init (arg=0x1ffeffdb58, class_type=0x4c96f20, properties=0x0) at Zend/zend_API.c:1755
0x0000000000a00f01 in object_init_ex (arg=0x1ffeffdb58, class_type=0x4c96f20) at Zend/zend_API.c:1794
0x0000000000b06622 in zend_throw_exception_zstr (exception_ce=0x4c96f20, message=0x4dec490, code=0) at Zend/zend_exceptions.c:828
0x0000000000b02b06 in zend_throw_exception (exception_ce=0x4c96f20, message=0x4dec370 "Class \"ReflectionFunction\" not found", code=0) at Zend/zend_exceptions.c:848
0x00000000009f86e6 in zend_throw_error (exception_ce=0x4c96f20, format=0xd32947 "%s") at Zend/zend.c:1714
0x00000000009dbed6 in zend_throw_or_error (fetch_type=512, exception_ce=0x0, format=0xcb23e2 "Class \"%s\" not found") at Zend/zend_execute_API.c:241
0x00000000009dc00c in report_class_fetch_error (class_name=0x4debdb0, fetch_type=512) at Zend/zend_execute_API.c:1635
0x00000000009dc1fe in zend_fetch_class_by_name (class_name=0x4debdb0, key=0x4debe20, fetch_type=512) at Zend/zend_execute_API.c:1724
0x0000000000a87861 in ZEND_NEW_SPEC_CONST_UNUSED_HANDLER (execute_data=0x4d91300) at Zend/zend_vm_execute.h:10482
0x0000000000a3a049 in execute_ex (ex=0x4d91300) at Zend/zend_vm_execute.h:56856
0x0000000000a3a3da in zend_execute (op_array=0x4deb1f0, return_value=0x0) at Zend/zend_vm_execute.h:61445
0x00000000009f920c in zend_execute_scripts (type=8, retval=0x0, file_count=3) at Zend/zend.c:1864
0x0000000000933561 in php_execute_script (primary_file=0x1fff0008f8) at main/main.c:2482
0x0000000000bc4b33 in do_cli (argc=2, argv=0x4bba7c0) at sapi/cli/php_cli.c:963
0x0000000000bc3cb9 in main (argc=2, argv=0x4bba7c0) at sapi/cli/php_cli.c:1337

PHP Version

PHP 8.3-dev

Operating System

Debian bullseye and sid

Activity

  1. ju1ius
    added
    Bug
    Status: Needs Triage
    on Jul 16, 2023

  2. ndossche commented on Jul 16, 2023

    @ndossche
    ndossche
    on Jul 16, 2023
    Member

    First of all, thanks for the nice reproducer!
    This seems to happen only under Valgrind, the --with-valgrind flag also doesn't seem to matter for my setup. Reproduces with a Clang build, works correctly with a GCC build.

    It infinite loops because it can't seem to find any class at all. i.e. it first wants to throw an exception because it can't find the ReflectionFunction class, then it fails in throwing the exception because it can't find the Error class. It wants to throw that too, so it comes into an infinite loop of wanting to throw an Error because it can't find the Error class.

    Indeed, executing this instead of your reproducer: new Error();, also causes a crash.
    I also get a bunch of "cannot load module" errors on startup, so that's likely related.

    Dumping out the class table shows error is in there, with the right length and hash. Yet zend_hash_find inside zend_lookup_class_ex returns NULL. EDIT: it's zend_string_equal_val that returns 0 in zend_hash_find.

  3. ndossche commented on Jul 16, 2023

    @ndossche
    ndossche
    on Jul 16, 2023
    Member

    I think this is a Clang bug regarding the Valgrind replacement function. Looking at its disassembly I see a pop rax, so that will overwrite the return value set by the memcmp...

    This is a workaround that works for me. I can do a PR later today:

    diff --git a/Zend/zend_string.c b/Zend/zend_string.c
index 549270690a..8957bd7cd5 100644
--- a/Zend/zend_string.c
+++ b/Zend/zend_string.c
@@ -392,9 +392,18 @@ ZEND_API void zend_interned_strings_switch_storage(bool request)
 # define NO_CALLER_SAVED_REGISTERS
 #endif
 
-ZEND_API bool ZEND_FASTCALL NO_CALLER_SAVED_REGISTERS I_REPLACE_SONAME_FNNAME_ZU(NONE,zend_string_equal_val)(const zend_string *s1, const zend_string *s2)
+static zend_never_inline ZEND_FASTCALL NO_CALLER_SAVED_REGISTERS void zend_string_equal_val_out_param(const zend_string *s1, const zend_string *s2, bool *out)
 {
-	return !memcmp(ZSTR_VAL(s1), ZSTR_VAL(s2), ZSTR_LEN(s1));
+	/* Clang's codegen has a bug where a "pop rax" will overwrite the return value in a "no_caller_saved_registers" function.
+	 * Work around this bug with a non-inlineable function with an out parameter. */
+	*out = !memcmp(ZSTR_VAL(s1), ZSTR_VAL(s2), ZSTR_LEN(s1));
+}
+
+ZEND_API bool ZEND_FASTCALL I_REPLACE_SONAME_FNNAME_ZU(NONE,zend_string_equal_val)(const zend_string *s1, const zend_string *s2)
+{
+	bool ret;
+	zend_string_equal_val_out_param(s1, s2, &ret);
+	return ret;
 }
 
 #ifdef POP_OPTIONS

    EDIT: and after applying that I get more of those Valgrind splats like in GH-9068 in release mode, haven't investigated why that is yet. In any case, the irony is that our the Valgrind replacement function is the one causing Valgrind to misbehave...

  4. ndossche
    added
    Category: Engine
    Status: Verified
    and removed
    Status: Needs Triage
    on Jul 16, 2023

  5. ndossche commented on Jan 9, 2024

    @ndossche
    ndossche
    on Jan 9, 2024
    Member

    This is also fixed by #13099.

  6. ndossche
    closed this as completedon Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugCategory: EngineStatus: Verified

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions