use-after-free in C2
Justin King
jcking at google.com
Thu Feb 16 15:27:43 UTC 2023
Yeah, I think that is it. Anybody have any idea what the intention is
supposed to be with PhaseIterGVN and it's copying by value of
Unique_Node_List and Node_Stack? Both of its constructors do this, one
happens to copy Compile::for_igvn() and ultimately modify it. Is it
supposed to be taking ownership, are any changes by PhaseIterGVN to either
supposed to be visible to the caller, or is it meant to be like
copy-on-write where changes are not visible to the caller?
On Thu, Feb 16, 2023 at 7:18 AM Justin King <jcking at google.com> wrote:
> Okay, continuing to look for the remaining surprises. I think I found it,
> but I need to verify. It looks like PhaseIterGVN is copying
> Compile::for_igvn() by value and modifying it either intentionally or
> unintentionally, triggering Arealloc. This results in Compile::for_igvn()
> now pointing to free'd memory. I'll see if I can verify.
>
> On Wed, Feb 15, 2023 at 9:05 AM Justin King <jcking at google.com> wrote:
>
>> I sent https://github.com/openjdk/jdk/pull/12577 and
>> https://github.com/openjdk/jdk/pull/12578 with their corresponding
>> created bugs. I'll continue looking for the others once those are in.
>>
>> On Tue, Feb 14, 2023 at 4:02 PM Justin King <jcking at google.com> wrote:
>>
>>> Looks like there is definitely a lingering use-after-free like bug,
>>> maybe multiple. One is related to the copying around of Unique_Node_List
>>> and the copy then modifying the list, causing a Arealloc, with the original
>>> (which is then used again) seeing the free'd block. I'll see about
>>> pinpointing the issues at a later date.
>>>
>>> On Tue, Feb 14, 2023 at 9:27 AM Vladimir Kozlov <
>>> vladimir.kozlov at oracle.com> wrote:
>>>
>>>> Hi Justin,
>>>>
>>>> Please file a bug for this issue. We need to keep records.
>>>>
>>>> Thanks,
>>>> Vladimir K
>>>>
>>>> On 2/10/23 12:42 PM, Justin King wrote:
>>>> > Hm. That size is still too big, I don't think the node is 224 bytes.
>>>> Let me double check again.
>>>> >
>>>> > On Fri, Feb 10, 2023 at 12:06 PM Justin King <jcking at google.com
>>>> <mailto:jcking at google.com>> wrote:
>>>> >
>>>> > Looks to be the temporary node created by clone_map is leaking. I
>>>> think there is a missing call to undo some of the
>>>> > work done by clone_map.
>>>> >
>>>> > ==3591618==ERROR: AddressSanitizer: use-after-poison on address
>>>> 0x6110000e20b8 at pc 0x7f7d8cac1875 bp
>>>> > 0x7f7d1028be20 sp 0x7f7d1028be18
>>>> > READ of size 4 at 0x6110000e20b8 thread T13
>>>> > #0 0x7f7d8cac1874 in
>>>> Unique_Node_List::remove_useless_nodes(VectorSet&)
>>>> src/hotspot/share/opto/node.cpp:2967
>>>> > #1 0x7f7d8cc90b03 in
>>>> PhaseRemoveUseless::PhaseRemoveUseless(PhaseGVN*, Unique_Node_List*,
>>>> Phase::PhaseNumber)
>>>> > src/hotspot/share/opto/phaseX.cpp:423
>>>> > #2 0x7f7d8b03853b in Compile::Compile(ciEnv*, ciMethod*,
>>>> int, Options, DirectiveSet*)
>>>> > src/hotspot/share/opto/compile.cpp:797
>>>> > #3 0x7f7d8ace8ece in C2Compiler::compile_method(ciEnv*,
>>>> ciMethod*, int, bool, DirectiveSet*)
>>>> > src/hotspot/share/opto/c2compiler.cpp:113
>>>> > #4 0x7f7d8b0507f8 in
>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)
>>>> > src/hotspot/share/compiler/compileBroker.cpp:2237
>>>> > #5 0x7f7d8b053e57 in CompileBroker::compiler_thread_loop()
>>>> src/hotspot/share/compiler/compileBroker.cpp:1916
>>>> > #6 0x7f7d8bc1f3a8 in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:710
>>>> > #7 0x7f7d8bc1f99f in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:689
>>>> > #8 0x7f7d8bc1f99f in JavaThread::run()
>>>> src/hotspot/share/runtime/javaThread.cpp:695
>>>> > #9 0x7f7d8d535b35 in Thread::call_run()
>>>> src/hotspot/share/runtime/thread.cpp:224
>>>> > #10 0x7f7d8cb7de8f in thread_native_entry
>>>> src/hotspot/os/linux/os_linux.cpp:737
>>>> > #11 0x7f7d8faa7fd3 in start_thread nptl/pthread_create.c:442
>>>> > #12 0x7f7d8fb2866b in clone3
>>>> ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
>>>> >
>>>> > 0x6110000e20b8 is located 56 bytes inside of 224-byte region
>>>> [0x6110000e2080,0x6110000e2160)
>>>> > allocated by thread T13 here:
>>>> > #0 0x7f7d8fcb89cf in __interceptor_malloc
>>>> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
>>>> > #1 0x7f7d8cb65b9a in os::malloc(unsigned long, MEMFLAGS,
>>>> NativeCallStack const&)
>>>> > src/hotspot/share/runtime/os.cpp:672
>>>> > #2 0x7f7d8a68c47e in Chunk::operator new(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum, unsigned long)
>>>> > src/hotspot/share/memory/arena.cpp:190
>>>> > #3 0x7f7d8a68c47e in Arena::grow(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum)
>>>> > src/hotspot/share/memory/arena.cpp:345
>>>> > #4 0x7f7d8cac1c44 in Arena::internal_amalloc(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum)
>>>> > src/hotspot/share/memory/arena.hpp:113
>>>> > #5 0x7f7d8cac1c44 in Arena::AmallocWords(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum)
>>>> > src/hotspot/share/memory/arena.hpp:140
>>>> > #6 0x7f7d8cac1c44 in Node::clone() const
>>>> src/hotspot/share/opto/node.cpp:495
>>>> > #7 0x7f7d8b980c38 in GraphKit::clone_map()
>>>> src/hotspot/share/opto/graphKit.cpp:727
>>>> > #8 0x7f7d8c538b44 in
>>>> LibraryCallKit::inline_unsafe_load_store(BasicType,
>>>> LibraryCallKit::LoadStoreKind,
>>>> > LibraryCallKit::AccessKind)
>>>> src/hotspot/share/opto/library_call.cpp:2599
>>>> > #9 0x7f7d8c5aad3e in LibraryCallKit::try_to_inline(int)
>>>> src/hotspot/share/opto/library_call.cpp:416
>>>> > #10 0x7f7d8c5ae34b in LibraryIntrinsic::generate(JVMState*)
>>>> src/hotspot/share/opto/library_call.cpp:116
>>>> > #11 0x7f7d8b4430e9 in Parse::do_call()
>>>> src/hotspot/share/opto/doCall.cpp:662
>>>> > #12 0x7f7d8cc4c1ef in Parse::do_one_bytecode()
>>>> src/hotspot/share/opto/parse2.cpp:2704
>>>> > #13 0x7f7d8cc1f3f6 in Parse::do_one_block()
>>>> src/hotspot/share/opto/parse1.cpp:1554
>>>> > #14 0x7f7d8cc207ce in Parse::do_all_blocks()
>>>> src/hotspot/share/opto/parse1.cpp:706
>>>> > #15 0x7f7d8cc2a214 in Parse::Parse(JVMState*, ciMethod*,
>>>> float) src/hotspot/share/opto/parse1.cpp:613
>>>> > #16 0x7f7d8acec235 in ParseGenerator::generate(JVMState*)
>>>> src/hotspot/share/opto/callGenerator.cpp:99
>>>> > #17 0x7f7d8b03738c in Compile::Compile(ciEnv*, ciMethod*,
>>>> int, Options, DirectiveSet*)
>>>> > src/hotspot/share/opto/compile.cpp:763
>>>> > #18 0x7f7d8ace8ece in C2Compiler::compile_method(ciEnv*,
>>>> ciMethod*, int, bool, DirectiveSet*)
>>>> > src/hotspot/share/opto/c2compiler.cpp:113
>>>> > #19 0x7f7d8b0507f8 in
>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)
>>>> > src/hotspot/share/compiler/compileBroker.cpp:2237
>>>> > #20 0x7f7d8b053e57 in CompileBroker::compiler_thread_loop()
>>>> src/hotspot/share/compiler/compileBroker.cpp:1916
>>>> > #21 0x7f7d8bc1f3a8 in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:710
>>>> > #22 0x7f7d8bc1f99f in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:689
>>>> > #23 0x7f7d8bc1f99f in JavaThread::run()
>>>> src/hotspot/share/runtime/javaThread.cpp:695
>>>> > #24 0x7f7d8d535b35 in Thread::call_run()
>>>> src/hotspot/share/runtime/thread.cpp:224
>>>> > #25 0x7f7d8cb7de8f in thread_native_entry
>>>> src/hotspot/os/linux/os_linux.cpp:737
>>>> > #26 0x7f7d8faa7fd3 in start_thread nptl/pthread_create.c:442
>>>> >
>>>> >
>>>> >
>>>> > On Thu, Feb 9, 2023 at 2:31 PM Justin King <jcking at google.com
>>>> <mailto:jcking at google.com>> wrote:
>>>> >
>>>> > Hm. It unfortunately does not show where it was poisoned, as
>>>> the Arena uses large chunks with multiple separate
>>>> > allocations per chunk. ASan only keeps track of malloc/free.
>>>> But if I change the Arena implementation when
>>>> > building under ASan to just use a right-sized chunk for each
>>>> request, it should be able to show us. I'll try
>>>> > that and bump this once I get something more definitive.
>>>> >
>>>> > On Thu, Feb 9, 2023 at 1:41 PM <dean.long at oracle.com <mailto:
>>>> dean.long at oracle.com>> wrote:
>>>> >
>>>> > Can ASan show where the memory was freed? We've had
>>>> crashes in the past (like JDK-8270028) that could be
>>>> > related to memory corruption or how ResourceArea recycles
>>>> memory. The allocation below seems to be using an
>>>> > Arena without a ResourceArea, but if some other code used
>>>> the same arena wrapped in a ResourceArea, then it
>>>> > seems like that could lead to potential problems.
>>>> >
>>>> > dl
>>>> >
>>>> > On 2/9/23 8:59 AM, Justin King wrote:
>>>> >> Hi,
>>>> >>
>>>> >> I was looking at instrumenting Arena again for ASan.
>>>> The WIP patch is
>>>> >> 047d4aa9a091cf5a84b9308454862e39666ca253
>>>> >> <
>>>> https://github.com/jcking/jdk/commit/047d4aa9a091cf5a84b9308454862e39666ca253>.
>>>> I ran back into the
>>>> >> suspicious logic in C2 <
>>>> https://bugs.openjdk.org/browse/JDK-8298984> where nodes are used
>>>> after calling
>>>> >> Arena::Afree. The first issue is present in
>>>> Node::destruct, which I fixed by moving the call to
>>>> >> Arena::Afree to the bottom of the function (addressed in
>>>> patch). The second issue came up after in
>>>> >> Compile::Compile, the stack trace is below. It looks
>>>> like there are residual freed nodes being operated
>>>> >> on? Maybe we are failing to unregister a temporary node
>>>> from the node list? Maybe related to clone_map
>>>> >> which returns SafePointNode?
>>>> >>
>>>> >> ==3146540==ERROR: AddressSanitizer: use-after-poison on
>>>> address 0x62d00996b370 at pc 0x7f9f93048335 bp
>>>> >> 0x7f9ed29fae20 sp 0x7f9ed29fae18
>>>> >> READ of size 4 at 0x62d00996b370 thread T13
>>>> >> #0 0x7f9f93048334 in
>>>> Unique_Node_List::remove_useless_nodes(VectorSet&)
>>>> >> src/hotspot/share/opto/node.cpp:2967
>>>> >> #1 0x7f9f932124b3 in
>>>> PhaseRemoveUseless::PhaseRemoveUseless(PhaseGVN*, Unique_Node_List*,
>>>> >> Phase::PhaseNumber) src/hotspot/share/opto/phaseX.cpp:423
>>>> >> #2 0x7f9f91621beb in Compile::Compile(ciEnv*,
>>>> ciMethod*, int, Options, DirectiveSet*)
>>>> >> src/hotspot/share/opto/compile.cpp:797
>>>> >> #3 0x7f9f912e37fe in
>>>> C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)
>>>> >> src/hotspot/share/opto/c2compiler.cpp:113
>>>> >> #4 0x7f9f91638e07 in
>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:2237
>>>> >> #5 0x7f9f9163bfd7 in
>>>> CompileBroker::compiler_thread_loop()
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:1916
>>>> >> #6 0x7f9f921e3eec in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:710
>>>> >> #7 0x7f9f921e434f in JavaThread::thread_main_inner()
>>>> src/hotspot/share/runtime/javaThread.cpp:689
>>>> >> #8 0x7f9f921e434f in JavaThread::run()
>>>> src/hotspot/share/runtime/javaThread.cpp:695
>>>> >> #9 0x7f9f93aa3f55 in Thread::call_run()
>>>> src/hotspot/share/runtime/thread.cpp:224
>>>> >> #10 0x7f9f9310144f in thread_native_entry
>>>> src/hotspot/os/linux/os_linux.cpp:737
>>>> >> #11 0x7f9f960a7fd3 in start_thread
>>>> nptl/pthread_create.c:442
>>>> >> #12 0x7f9f9612866b in clone3
>>>> ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
>>>> >>
>>>> >> 0x62d00996b370 is located 20336 bytes inside of
>>>> 32744-byte region [0x62d009966400,0x62d00996e3e8)
>>>> >> allocated by thread T13 here:
>>>> >> #0 0x7f9f962b89cf in __interceptor_malloc
>>>> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
>>>> >> #1 0x7f9f930e915a in os::malloc(unsigned long,
>>>> MEMFLAGS, NativeCallStack const&)
>>>> >> src/hotspot/share/runtime/os.cpp:672
>>>> >> #2 0x7f9f90c8a08a in Chunk::operator new(unsigned
>>>> long, AllocFailStrategy::AllocFailEnum, unsigned
>>>> >> long) src/hotspot/share/memory/arena.cpp:190
>>>> >> #3 0x7f9f90c8a08a in Arena::grow(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum)
>>>> >> src/hotspot/share/memory/arena.cpp:325
>>>> >> #4 0x7f9f932109f5 in
>>>> Arena::internal_amalloc(unsigned long, AllocFailStrategy::AllocFailEnum)
>>>> >> src/hotspot/share/memory/arena.hpp:113
>>>> >> #5 0x7f9f932109f5 in Arena::Amalloc(unsigned long,
>>>> AllocFailStrategy::AllocFailEnum)
>>>> >> src/hotspot/share/memory/arena.hpp:133
>>>> >> #6 0x7f9f932109f5 in NodeHash::NodeHash(Arena*,
>>>> unsigned int) src/hotspot/share/opto/phaseX.cpp:68
>>>> >> #7 0x7f9f932293c7 in
>>>> PhaseValues::PhaseValues(Arena*, unsigned int)
>>>> src/hotspot/share/opto/phaseX.cpp:697
>>>> >> #8 0x7f9f9161f678 in PhaseGVN::PhaseGVN(Arena*,
>>>> unsigned int) src/hotspot/share/opto/phaseX.hpp:415
>>>> >> #9 0x7f9f9161f678 in Compile::Compile(ciEnv*,
>>>> ciMethod*, int, Options, DirectiveSet*)
>>>> >> src/hotspot/share/opto/compile.cpp:714
>>>> >> #10 0x7f9f912e37fe in
>>>> C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)
>>>> >> src/hotspot/share/opto/c2compiler.cpp:113
>>>> >> #11 0x7f9f91638e07 in
>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:2237
>>>> >> #12 0x7f9f9163bfd7 in
>>>> CompileBroker::compiler_thread_loop()
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:1916
>>>> >> #13 0x7f9f921e3eec in
>>>> JavaThread::thread_main_inner() src/hotspot/share/runtime/javaThread.cpp:710
>>>> >> #14 0x7f9f921e434f in
>>>> JavaThread::thread_main_inner() src/hotspot/share/runtime/javaThread.cpp:689
>>>> >> #15 0x7f9f921e434f in JavaThread::run()
>>>> src/hotspot/share/runtime/javaThread.cpp:695
>>>> >> #16 0x7f9f93aa3f55 in Thread::call_run()
>>>> src/hotspot/share/runtime/thread.cpp:224
>>>> >> #17 0x7f9f9310144f in thread_native_entry
>>>> src/hotspot/os/linux/os_linux.cpp:737
>>>> >> #18 0x7f9f960a7fd3 in start_thread
>>>> nptl/pthread_create.c:442
>>>> >>
>>>> >> Thread T13 created by T1 here:
>>>> >> #0 0x7f9f96249726 in __interceptor_pthread_create
>>>> >>
>>>> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
>>>> >> #1 0x7f9f93102d88 in os::create_thread(Thread*,
>>>> os::ThreadType, unsigned long)
>>>> >> src/hotspot/os/linux/os_linux.cpp:888
>>>> >> #2 0x7f9f91693d93 in
>>>> CompilerThread::CompilerThread(CompileQueue*, CompilerCounters*)
>>>> >> src/hotspot/share/compiler/compilerThread.cpp:34
>>>> >> #3 0x7f9f91625c7c in
>>>> CompileBroker::make_thread(CompileBroker::ThreadType, _jobject*,
>>>> CompileQueue*,
>>>> >> AbstractCompiler*, JavaThread*)
>>>> src/hotspot/share/compiler/compileBroker.cpp:842
>>>> >> #4 0x7f9f91628f71 in
>>>> CompileBroker::init_compiler_threads()
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:943
>>>> >> #5 0x7f9f9162a464 in
>>>> CompileBroker::compilation_init_phase1(JavaThread*)
>>>> >> src/hotspot/share/compiler/compileBroker.cpp:654
>>>> >> #6 0x7f9f93adc3a4 in
>>>> Threads::create_vm(JavaVMInitArgs*, bool*)
>>>> src/hotspot/share/runtime/threads.cpp:701
>>>> >> #7 0x7f9f92465b51 in JNI_CreateJavaVM_inner
>>>> src/hotspot/share/prims/jni.cpp:3588
>>>> >> #8 0x7f9f92465b51 in JNI_CreateJavaVM
>>>> src/hotspot/share/prims/jni.cpp:3674
>>>> >> #9 0x7f9f968d2e25 in InitializeJVM
>>>> src/java.base/share/native/libjli/java.c:1459
>>>> >> #10 0x7f9f968d2e25 in JavaMain
>>>> src/java.base/share/native/libjli/java.c:413
>>>> >> #11 0x7f9f968db708 in ThreadJavaMain
>>>> src/java.base/unix/native/libjli/java_md.c:650
>>>> >> #12 0x7f9f960a7fd3 in start_thread
>>>> nptl/pthread_create.c:442
>>>> >>
>>>> >> Thread T1 created by T0 here:
>>>> >> #0 0x7f9f96249726 in __interceptor_pthread_create
>>>> >>
>>>> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
>>>> >> #1 0x7f9f968dd3a1 in CallJavaMainInNewThread
>>>> src/java.base/unix/native/libjli/java_md.c:691
>>>> >> #2 0x7f9f968d822d in ContinueInNewThread
>>>> src/java.base/share/native/libjli/java.c:2280
>>>> >> #3 0x7f9f968d96ae in JLI_Launch
>>>> src/java.base/share/native/libjli/java.c:340
>>>> >> #4 0x5594a81c337c in main
>>>> src/java.base/share/native/launcher/main.c:166
>>>> >> #5 0x7f9f96046189 in __libc_start_call_main
>>>> ../sysdeps/nptl/libc_start_call_main.h:58
>>>> >>
>>>> >> --
>>>> >>
>>>> >> Google Logo
>>>> >> Justin King
>>>> >> Software Engineer
>>>> >> jcking at google.com <mailto:jcking at google.com>
>>>> >>
>>>> >>
>>>> >>
>>>> >>
>>>> >
>>>> >
>>>> > --
>>>> >
>>>> > Google Logo
>>>> > Justin King
>>>> > Software Engineer
>>>> > jcking at google.com <mailto:jcking at google.com>
>>>> >
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> >
>>>> > Google Logo
>>>> > Justin King
>>>> > Software Engineer
>>>> > jcking at google.com <mailto:jcking at google.com>
>>>> >
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> >
>>>> > Google Logo
>>>> > Justin King
>>>> > Software Engineer
>>>> > jcking at google.com <mailto:jcking at google.com>
>>>> >
>>>> >
>>>> >
>>>>
>>>
>>>
>>> --
>>>
>>> [image: Google Logo]
>>> Justin King
>>> Software Engineer
>>> jcking at google.com
>>>
>>>
>>
>> --
>>
>> [image: Google Logo]
>> Justin King
>> Software Engineer
>> jcking at google.com
>>
>>
>
> --
>
> [image: Google Logo]
> Justin King
> Software Engineer
> jcking at google.com
>
>
--
[image: Google Logo]
Justin King
Software Engineer
jcking at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-compiler-dev/attachments/20230216/ce253051/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3999 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://mail.openjdk.org/pipermail/hotspot-compiler-dev/attachments/20230216/ce253051/smime-0001.p7s>
More information about the hotspot-compiler-dev
mailing list