ChakraCore: ImmutableList undefined behavior

OS: Ubuntu 22.04 x86-64 Compiler: Clang ~14.0.0~12.0.1 CMake build type: RelWithDebInfo

Bytecode generation for verification segfaults:

$ ./ch -GenerateLibraryByteCodeHeader -LdChakraLib -JsBuiltIn /path/to/ChakraCore/tools/../lib/Runtime/Library/InJavascript/Array_prototype.js
Segmentation fault (core dumped)

The crash comes from code in ByteCodeSerializer.cpp, though it might also be related to ImmutableList, or something else.

$ gdb --args ./ch -GenerateLibraryByteCodeHeader -LdChakraLib -JsBuiltIn /local/petr/ChakraCore/tools/../lib/Runtime/Library/InJavascript/Array_prototype.js
GNU gdb (Ubuntu 12.0.90-0ubuntu1) 12.0.90
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./ch...
(gdb) r
Starting program: /local/petr/ChakraCore/build-test/ch -GenerateLibraryByteCodeHeader -LdChakraLib -JsBuiltIn /local/petr/ChakraCore/tools/../lib/Runtime/Library/InJavascript/Array_prototype.js
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ff7f43cf640 (LWP 5873)]
[New Thread 0x7ff7f3b9f640 (LWP 5874)]
[New Thread 0x7ff7f339e640 (LWP 5875)]

Thread 1 "ch" received signal SIGSEGV, Segmentation fault.
regex::ImmutableList<Js::BufferBuilder*>::ReverseCurrentList (this=<optimized out>) at /local/petr/ChakraCore/lib/Common/DataStructures/ImmutableList.h:387
387	                auto next = current->next;
(gdb) bt
#0  regex::ImmutableList<Js::BufferBuilder*>::ReverseCurrentList (this=<optimized out>)
    at /local/petr/ChakraCore/lib/Common/DataStructures/ImmutableList.h:387
#1  Js::ByteCodeBufferBuilder::AddFunction (this=0x7fffffffd9f0, this@entry=0x7fffffffdd48, 
    builder=..., function=0x7ff7f2b65380, srcInfo=srcInfo@entry=0x7ff7f3ba1e40, cache=cache@entry=0x0)
    at /local/petr/ChakraCore/lib/Runtime/ByteCode/ByteCodeSerializer.cpp:2440

Failure doesn’t exist in Debug mode. I’ve tried to build some files with Debug, but that leads to undefined symbols (as more inlining removes some of the calls), maybe I’ll have some more success with Release mode. ~Worth looking into earlier Clang versions, we don’t have this failure on Ubuntu 20, where package manager doesn’t have Clang 14.~ We didn’t have this on Ubuntu 20, which has Clang 10 in the package repo. I can narrow down it to Clang 12 vs Clang 11, latter works.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 19 (12 by maintainers)

Commits related to this issue

Most upvoted comments

Yeah, immutable list modifying on append is pretty hilarious. This is basically a C linked list written with C++ classes.

After thinking about this for a bit, I think we can make IsEmpty a static function - it would still be able to do nullptr checks, though not from within a class on instance pointer. Instead of x.isEmpty it would be isEmpty(x). And it would still be inlineable.

To future proof the code and avoid any submarine bugs we ought to delete IsEmpty, and re-factor throughout.

I note however that in a couple of places this may require changes at function call sites, currently several Methods in that class are designed to be able to be called on a nullptr OR a valid item; in light of modern C++ optimising away this==nullptr checks I think we should refactor and check we’re not calling any methods on nullptrs.