Correctly handle primitive VTs in System.arraycopy #17048

ehrenjulzert · 2023-03-28T15:32:16Z

Create VM_ValueTypeHelpers::copyFlattenableArray
function and call it instead of
VM_ArrayCopyHelpers::referenceArrayCopy when
copying flattened or primitive arrays
Throw a NPE instead of an ASE in
doReferenceArrayCopy when an attempt is made to
copy a null object into an array of primitive
value types

ehrenjulzert · 2023-03-28T15:44:19Z

One thing I'm unsure of is how/if impl_jitReferenceArrayCopy should be changed to throw a NPE, since it seems to not differentiate between different kinds of errors (always returns (void*)-1 when there's an exception). @hangshao0 do you know who I could ask about that?

hangshao0 · 2023-03-28T18:34:48Z

runtime/gc_modron_startup/arrayCopy.cpp

@@ -75,7 +76,11 @@ typeCheckArrayStore(J9VMThread *vmThread, J9Object *object, J9IndexableObject *a
 				return (0 != instanceOfOrCheckCast(storedClazz, componentType));
 			}
 		}
+	} else if (J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(componentType)) {


The new code should be inside #if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)

added in 92cb616

hangshao0 · 2023-03-28T18:37:22Z

runtime/vm/BytecodeInterpreter.hpp

+
+						if (J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(destComponentClass) &&
+							!J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(srcComponentClass) &&
+							(NULL == J9JAVAARRAYOFOBJECT_LOAD(_currentThread, srcObject, srcStart))) {


It seems that only one element in the array is loaded here and compared against NULL.

I don't see the check of flatten vs not flatten on the srcComponentClass. Does the NPE apply to the flattened case ?

Only the first element being copied needs to be checked, because if the first element is both non-null and nullable then an ASE should be thrown instead (This is how it works on the RI)

Oh wait actually you're right, I didn't take into account the fact that primitive VTs could be inside an array of Objects. If you try to copy something like new Object[] {new VTInt(), null, null} into an array of VTInts then you will still get a NPE, whereas new Object[] {new Object(), null, null} will get you an ASE.

I can change the code to just check if the element at the index returned by referenceArrayCopy is null (since referenceArrayCopy returns the index at which an error occurred)

check if the element at the index returned by referenceArrayCopy is null

This should work.

Ok I fixed this issue in 92cb616

I also added a check to make sure the source array is not flattened

hangshao0 · 2023-03-28T18:37:47Z

runtime/vm/BytecodeInterpreter.hpp

@@ -3030,6 +3043,7 @@ class INTERPRETER_CLASS
 							buildInternalNativeStackFrame(REGISTER_ARGS);
 							rc = THROW_ARRAY_STORE;
 						} else {
+


Please remove the empty line change.

fixed in 92cb616

hangshao0 · 2023-03-28T18:41:33Z

runtime/vm/BytecodeInterpreter.hpp

+						J9Class *destComponentClass = ((J9ArrayClass *)J9OBJECT_CLAZZ(_currentThread, destObject))->componentType;
+
+						if (J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(destComponentClass) &&
+							!J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(srcComponentClass) &&


Do we need to check if srcComponentClass is primitive VT ? It could be an array of Object and srcComponentClass is java.lang.Object.

Yeah I suppose that check is unnecessary since I'm already checking if the problem element is null on the next line

removed in 92cb616

hangshao0 · 2023-03-29T17:09:29Z

One thing I'm unsure of is how/if impl_jitReferenceArrayCopy should be changed to throw a NPE, since it seems to not differentiate between different kinds of errors (always returns (void*)-1 when there's an exception). @hangshao0 do you know who I could ask about that?

Do you want a different return code from the JIT helper impl_jitReferenceArrayCopy for the NPE case (when copying a null object into an array of primitive value types) ? @hzongaro @a7ehuo.

hangshao0 · 2023-03-30T14:50:03Z

runtime/gc_modron_startup/arrayCopy.cpp

@@ -64,8 +64,9 @@ alwaysCallReferenceArrayCopyHelper(J9JavaVM *javaVM)
 static bool
 typeCheckArrayStore(J9VMThread *vmThread, J9Object *object, J9IndexableObject *arrayObj)
 {
+	J9Class *componentType = ((J9ArrayClass *)J9OBJECT_CLAZZ(vmThread, arrayObj))->componentType;


For the case object and arrayObj are both NULL, this function returned true before this change. Now it will segfault.

I could add a bit at the top to return true if both object and arrayObj are null, but I feel like that's not intended behaviour. If the arrayObj is null then there shouldn't be any legal stores to it, so really I should be returning false in that case.

hangshao0 · 2023-03-30T14:52:35Z

runtime/vm/BytecodeInterpreter.hpp

-					if (-1 != value) {
-						buildInternalNativeStackFrame(REGISTER_ARGS);
-						rc = THROW_ARRAY_STORE;
+					I_32 errorIndex = VM_ArrayCopyHelpers::referenceArrayCopy(_currentThread, srcObject, srcStart, destObject, destStart, elementCount);


I see the change here and in FastJNI_java_lang_System.cpp are duplicated. Can the change be made inside the VM helper VM_ArrayCopyHelpers::referenceArrayCopy() ?

Good point, I would have to change the return value of VM_ArrayCopyHelpers::referenceArrayCopy to a VM_BytecodeAction but I suppose it would be worth it.

hangshao0 · 2023-03-30T17:28:20Z

@dmitripivkine Can you review the change in the GC code ?

amicic · 2023-03-30T22:02:27Z

where did we actually fail during the copy in the existing code (I understand if we failed, that we unconditionally threw ASE)?

edit: I guess we actually succeeded in copying?

dmitripivkine · 2023-03-31T13:12:34Z

I am not a VT expert, so please correct me in the case of misunderstanding.

I hope original claim that reference array of VTs can not have NULL elements is correct. I don't think it is relevant from GC point of view.
VT class can not have subclass and has depth 1 with j.l.Object as superclass. Am I reading code correctly that j.l.Objects elements cat not be copied to VT elements? If so, and VT->VT allowed only how source array can have NULLs? And if it a case check at element copy time is not necessary. Coping elements with types not allowed should be handled on front of copy operation
If somehow there is no other way to control delivery of NULL elements to VT array as in coping time the question is why this handling done for forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex() copy function only (where typeCheckArrayStore() is called) and rest of copy functions in the table are ignored? There is might be a case that this is only function can be used to copy VT array elements (happen to be or there is deterministic implementation logic?). In this case I think change can be accepted but should be more explicit explanation why so (at least comment)

ehrenjulzert · 2023-03-31T18:08:37Z

@amicic Yes, in the original code we succeeded in copying, which was an error. Originally we always allowed null to be stored in any reference type array, which let us to actually do an illegal operation by storing null in an array of primitive value types. To fix this I added some code in typeCheckArrayStore that would return an error if we attempt to store null in an array of primitive value types, but unfortunately all errors thrown by typeCheckArrayStore are ASE exceptions, and in this specific case we need to throw a NPE. So to get it to throw a NPE I'm also changing how the errors are handled in doReferenceArrayCopy.

ehrenjulzert · 2023-03-31T21:07:01Z

@dmitripivkine

I hope original claim that reference array of VTs can not have NULL elements is correct

Yup, this is correct. Primitive VTs are not nullable.

j.l.Objects elements can not be copied to VT elements

This isn't true, j.l.Object elements can be copied into a VT array of type T[], but only if they are not null and they can be downcast from Object to T. Consider the following 2 cases:

primitive static class VTInt {
  int i;
  VTInt(int i) {
    this.i = i;
  }
}

VTInt[] vtiAry = new VTInt[3];

// Case 1: this will work because you are allowed to copy Objects into vtiAry so long as they can be downcast to the type VTInt
Object[] oarr = new Object[] { new VTInt(1), new VTInt(1), new VTInt(1) };
System.arraycopy(oarr, 0, vtiAry, 0, 3);

// Case 2: this arraycopy call will throw a NPE because the last element in oarr is null
Object[] oarr = new Object[] { new VTInt(1), new VTInt(1), null };
System.arraycopy(oarr, 0, vtiAry, 0, 3);

why this handling done for forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex() copy function only

The table is defined here:

openj9/runtime/gc_modron_startup/arrayCopy.cpp

Lines 267 to 276 in e656d2e

    
           /* Forward copies with type check on each element (check for ArrayStoreException) */ 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_illegal] = copyVariantUndefinedIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_none] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_always] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_oldcheck] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_cardmark] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_cardmark_incremental] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_cardmark_and_oldcheck] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex; 
        
           table->forwardReferenceArrayCopyWithCheckIndex[j9gc_modron_wrtbar_satb] = forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex;

The only two functions in the table are forwardReferenceArrayCopyWithCheckAndAlwaysWrtbarIndex and copyVariantUndefinedIndex. copyVariantUndefinedIndex does nothing except throw an error:

openj9/runtime/gc_modron_startup/arrayCopy.cpp

Lines 241 to 245 in e656d2e

    
           copyVariantUndefinedIndex(J9VMThread *vmThread, J9IndexableObject *srcObject, J9IndexableObject *destObject, I_32 srcIndex, I_32 destIndex, I_32 lengthInSlots) 
        
           {	 
        
           	Assert_MM_unreachable(); 
        
           	return srcIndex; 
        
           }

hangshao0 · 2023-04-03T15:48:45Z

@ehrenjulzert Does RI allow both System.arraycopy(oarr, 0, vtiAry, 0, 3) and System.arraycopy(vtiAry, 0, oarr, 0, 3), regardless of flattening (inlining) enabled or disabled ?

ehrenjulzert · 2023-04-03T16:19:12Z

Yes, flattening doesn't matter

hangshao0 · 2023-04-03T21:26:30Z

Looks like RI allows copying between flattened array and object (non-flattened) array. Does OpenJ9 give the correct result if arrayCopy is done from object array to flattened array and from flattened array to object array ? If not, more change is needed here.

ehrenjulzert · 2023-04-03T23:19:59Z

Does OpenJ9 give the correct result if arrayCopy is done from object array to flattened array and from flattened array to object array ?

Good catch, it doesn't work. When I was testing it earlier I was just looking at whether or not there was an exception, I wasn't looking at the values of the arrays themselves. When I try to copy the flattened array into the object array the values are treated as pointers, and vice versa. I'll have to add some logic to check if we are copying from a flattened to unflattened array or vice versa.

hzongaro · 2023-04-06T14:12:05Z

Do you want a different return code from the JIT helper impl_jitReferenceArrayCopy for the NPE case (when copying a null object into an array of primitive value types) ?

If I'm understanding correctly, some platforms currently check whether the result is non-zero, and if so, throw an ArrayStoreException, while others check whether the result is -1. We'll need to sort out how we'll want to handle the possibility of NullPointerException. I'll update you as soon as I have more information.

ehrenjulzert · 2023-04-13T19:28:46Z

My update in 740909d removed all the GC code changes, and added flattened array handling to arraycopy using a new helper function, copyFlattenableArray.

hangshao0 · 2023-04-13T19:51:58Z

runtime/vm/ValueTypeHelpers.hpp

+				destIndex--;
+
+				/* No type checks required since srcObject == destObject */
+				j9object_t copyObject = loadFlattenableArrayElement(currentThread, objectAccessBarrier, objectAllocate, srcObject, srcIndex, true);


It is possible that loadFlattenableArrayElement() failed in the fast path and the returned copyObject is NULL.

Added checks for this in 5db20fe

hangshao0 · 2023-04-13T21:00:09Z

Talked to @ehrenjulzert, there is a possible optimization when copying from flattened array to flattened array. There is no need to allocate the copyObject and check if the element in the array is null.

hzongaro · 2023-04-14T13:59:51Z

Ehren @ehrenjulzert, just to follow up on your question:

Do you want a different return code from the JIT helper impl_jitReferenceArrayCopy for the NPE case (when copying a null object into an array of primitive value types) ?

Annabelle, Vijay, Daryl and I discussed this offline, and decided that, for now at least, the JIT will avoid using impl_jitReferenceArrayCopy in situations where a NullPointerException might be thrown - in such situations it will fall back to using the actual System.arraycopy. So there's no need for impl_jitReferenceArrayCopy to return a distinct failure result for now - it should continue to return -1.

ehrenjulzert · 2023-04-27T19:20:33Z

In the latest commit I updated copyFlattenableArray to correctly handle allocation failures in loadFlattenableArrayElement, and also change the code to call primitiveArrayCopy when copying a flattened array into another flattened array (which should be much faster than calling copyFlattenableArray)

hangshao0 · 2023-04-28T15:59:23Z

runtime/vm/BytecodeInterpreter.hpp

+					J9Class *destClazz = J9OBJECT_CLAZZ(_currentThread, destObject);
+					J9Class *destComponentClass = ((J9ArrayClass *)destClazz)->componentType;
+
+					if (J9_IS_J9CLASS_FLATTENED(srcClazz) && J9_IS_J9CLASS_FLATTENED(destClazz) && (srcClazz == destClazz)) {


In the case srcClazz and destClazz are both flattened, we can directly throw Array Store Exception if srcClazz != destClazz. The current code will goes into referenceArrayCopy to throw the expectation, which is not obvious. I suggest moving (srcClazz == destClazz) to the next line like:

if (J9_IS_J9CLASS_FLATTENED(srcClazz) && J9_IS_J9CLASS_FLATTENED(destClazz) { if (srcClazz == destClazz) {

Throw the expectation if they are not the same class.

added in 25f3eed

hangshao0 · 2023-04-28T16:33:35Z

runtime/vm/ValueTypeHelpers.hpp

+				j9object_t copyObject = loadFlattenableArrayElement(currentThread, objectAccessBarrier, objectAllocate, srcObject, srcIndex, true);
+
+				/* Check for an allocation failure (only possible if srcClazz is flattened) */
+				if ((NULL == copyObject) && J9_IS_J9CLASS_FLATTENED(srcClazz)) {


If it is only possible if srcClazz is flattened, then you should assert J9_IS_J9CLASS_FLATTENED(srcClazz) is true inside if (NULL == copyObject)

What I mean by only possible if srcClazz is flattened is that when copyObject is NULL, 2 things are possible:

1: The flattenable array element at that index is actually NULL
Or
2: There was an allocation failure

Allocation failures are only possible when srcClazz is flattened, and so we need to make sure both (NULL == copyObject) and J9_IS_J9CLASS_FLATTENED(srcClazz) are true.

I guess I should update the comment to make that more clear?

Yes, please update the comment.

updated in 25f3eed

runtime/vm/ValueTypeHelpers.hpp

hangshao0 · 2023-04-28T16:41:08Z

runtime/vm/ValueTypeHelpers.hpp

+					if (!VM_VMHelpers::objectArrayStoreAllowed(currentThread, destObject, copyObject)) {
+						return -1;
+					}
+					if (J9_IS_J9CLASS_PRIMITIVE_VALUETYPE(destComponentClass) && (NULL == copyObject)) {


I suggest to do the NPE check before the objectArrayStoreAllowed() check.

moved in 25f3eed

hangshao0 · 2023-04-28T16:43:57Z

runtime/vm/ValueTypeHelpers.hpp

+			UDATA srcDepth = J9CLASS_DEPTH(srcClazz);
+			UDATA destDepth = J9CLASS_DEPTH(destClazz);
+			if ((srcDepth <= destDepth) || (destClazz->superclasses[srcDepth] != srcClazz)) {
+				typeChecksRequired = true;


typeChecksRequired is only used at line 639, so this block to set typeChecksRequired can be moved into the else block at line 628.

moved in 25f3eed

hangshao0 · 2023-05-01T14:37:08Z

runtime/vm/BytecodeInterpreter.hpp

+
+					if (J9_IS_J9CLASS_FLATTENED(srcClazz) && J9_IS_J9CLASS_FLATTENED(destClazz)) {
+						if (srcClazz == destClazz) {
+							UDATA elementSize = J9ARRAYCLASS_GET_STRIDE(srcClazz);


Can you add a comment this only works for contiguous flattened arrays for now as GC does not support the discontiguous case.

added in 3eecee5

hangshao0 · 2023-05-01T17:55:49Z

@dmitripivkine Can you review the change in the GC code ?

There is no GC change in this PR anymore. But you are still welcome to review if you want. @dmitripivkine

tajila · 2023-05-03T19:55:45Z

runtime/vm/ValueTypeHelpers.hpp

+			* Decide if type checks are required
+			* Type checks are not required only if both classes are the same, or the source class is a subclass of the dest class
+			*/
+			if (srcClazz != destClazz) {


can you use isSameOrSuperClassOf?

Yeah I can, good point

updated in e1d42a4

tajila · 2023-05-10T18:26:47Z

runtime/vm/BytecodeInterpreter.hpp

+					J9Class *destClazz = J9OBJECT_CLAZZ(_currentThread, destObject);
+					J9Class *destComponentClass = ((J9ArrayClass *)destClazz)->componentType;
+
+					if (J9_IS_J9CLASS_FLATTENED(srcClazz) && J9_IS_J9CLASS_FLATTENED(destClazz)) {


I think this has to be, if both are flattened and dont contain any object references. If there is an object ref, you need to do a barrier load/store

We should consider making a VM_ArrayCopyHelpers::vtArrayCopy that does a batch barrier. @dmitripivkine

You can check if the class has references between doing J9_ARE_ANY_BITS_SET(clazz->classFlags, J9ClassHasReferences)

Ok I updated the check in 9f31ca4

tajila · 2023-05-15T21:17:35Z

runtime/vm/BytecodeInterpreter.hpp

+					J9Class *srcComponentClass = ((J9ArrayClass *)srcClazz)->componentType;
+					J9Class *destComponentClass = ((J9ArrayClass *)destClazz)->componentType;
+
+					if (J9_IS_J9CLASS_FLATTENED(srcClazz) && J9_IS_J9CLASS_FLATTENED(destClazz) && !J9_ARE_ANY_BITS_SET(srcComponentClass->classFlags, J9ClassHasReferences) && !J9_ARE_ANY_BITS_SET(destComponentClass->classFlags, J9ClassHasReferences)) {


sorry, I should have suggested J9_ARE_NO_BITS_SET(... so you dont have to add the !

updated in e2d43b1

tajila

Just one minor change

hangshao0 · 2023-05-15T21:21:25Z

The #if defined(J9VM_OPT_VALHALLA_VALUE_TYPES) in this PR should be changed to the new flag J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES now as #17394 is merged.

hangshao0 · 2023-05-15T21:24:13Z

In the future, all the code related to value objects should be added inside #if defined(J9VM_OPT_VALHALLA_VALUE_TYPES). All the code related to primitive value types should be added inside #if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES).

- Create VM_ValueTypeHelpers::copyFlattenableArray function and call it instead of VM_ArrayCopyHelpers::referenceArrayCopy when copying flattened or primitive arrays - Throw a NPE instead of an ASE in doReferenceArrayCopy when an attempt is made to copy a null object into an array of primitive value types for eclipse-openj9#13182 Signed-off-by: Ehren Julien-Neitzert <ehren.julien-neitzert@ibm.com>

ehrenjulzert · 2023-05-16T15:03:14Z

Ok, I changed the #if defined to J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES

tajila · 2023-05-16T21:30:20Z

Jenkins test sanity win jdk8

tajila · 2023-05-16T21:30:26Z

Jenkins test sanity,extended xlinuxval jdknext

hangshao0 reviewed Mar 28, 2023

View reviewed changes

hangshao0 added comp:vm project:valhalla Used to track Project Valhalla related work labels Mar 28, 2023

ehrenjulzert force-pushed the InlineTypeArray_fix branch from bc29c33 to 92cb616 Compare March 29, 2023 22:24

hangshao0 reviewed Mar 30, 2023

View reviewed changes

hangshao0 requested a review from dmitripivkine March 30, 2023 17:27

ehrenjulzert mentioned this pull request Apr 11, 2023

Remove Fast_java_lang_System_arraycopy #17140

Merged

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 92cb616 to 740909d Compare April 13, 2023 19:19

hangshao0 reviewed Apr 13, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 740909d to 5db20fe Compare April 13, 2023 20:41

hzongaro mentioned this pull request Apr 14, 2023

JIT generated code must avoid using arraycopy instruction if a NullPointerException could result #17196

Closed

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 5db20fe to 392d059 Compare April 27, 2023 19:12

hangshao0 reviewed Apr 28, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch 2 times, most recently from 7337a0e to 25f3eed Compare April 28, 2023 21:30

hangshao0 reviewed May 1, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 25f3eed to 3eecee5 Compare May 1, 2023 17:50

hangshao0 approved these changes May 1, 2023

View reviewed changes

hangshao0 requested a review from tajila May 1, 2023 17:53

tajila reviewed May 3, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 3eecee5 to e1d42a4 Compare May 4, 2023 19:40

tajila reviewed May 10, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch from e1d42a4 to 9f31ca4 Compare May 11, 2023 18:27

tajila reviewed May 15, 2023

View reviewed changes

tajila approved these changes May 15, 2023

View reviewed changes

ehrenjulzert force-pushed the InlineTypeArray_fix branch from 9f31ca4 to e2d43b1 Compare May 16, 2023 15:01

tajila merged commit 7d5d62a into eclipse-openj9:master May 17, 2023
8 checks passed

Correctly handle primitive VTs in System.arraycopy #17048

Correctly handle primitive VTs in System.arraycopy #17048

Conversation

ehrenjulzert commented Mar 28, 2023 • edited

ehrenjulzert commented Mar 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehrenjulzert Mar 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangshao0 commented Mar 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangshao0 commented Mar 30, 2023

amicic commented Mar 30, 2023 • edited

dmitripivkine commented Mar 31, 2023

ehrenjulzert commented Mar 31, 2023

ehrenjulzert commented Mar 31, 2023

hangshao0 commented Apr 3, 2023

ehrenjulzert commented Apr 3, 2023

hangshao0 commented Apr 3, 2023 • edited

ehrenjulzert commented Apr 3, 2023

hzongaro commented Apr 6, 2023

ehrenjulzert commented Apr 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangshao0 commented Apr 13, 2023

hzongaro commented Apr 14, 2023

ehrenjulzert commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangshao0 commented May 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tajila May 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tajila left a comment

Choose a reason for hiding this comment

hangshao0 commented May 15, 2023

hangshao0 commented May 15, 2023

ehrenjulzert commented May 16, 2023

tajila commented May 16, 2023

tajila commented May 16, 2023

ehrenjulzert commented Mar 28, 2023 •

edited

ehrenjulzert Mar 28, 2023 •

edited

amicic commented Mar 30, 2023 •

edited

hangshao0 commented Apr 3, 2023 •

edited

tajila May 15, 2023 •

edited