Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix instructions for some packets #42

Merged
merged 2 commits into from
Oct 29, 2022
Merged

Fix instructions for some packets #42

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30188ee
Fix instructions for some packets
mat-1 Oct 27, 2022
d156c88
fix lengths sometimes being prefixed by 0x
mat-1 Oct 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 55 additions & 12 deletions burger/toppings/packetinstructions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from jawa.transforms import simple_swap

from .topping import Topping
from burger.util import InvokeDynamicInfo, REF_invokeStatic
from burger.util import InvokeDynamicInfo, REF_invokeStatic, get_enum_constants

SUB_INS_EPSILON = .01
PACKETBUF_NAME = "packetbuffer" # Used to specially identify the PacketBuffer we care about
Expand Down Expand Up @@ -420,6 +420,15 @@ def operations(classloader, cf, classes, verbose, method, arg_names, special_fie
var=array,
value=value))

elif instruction == 'putfield':
# Set a field in an object
value = stack.pop()
obj = stack.pop()
operations.append(Operation(instruction.pos, "putfield",
field=operands[0].name,
obj=obj,
value=value))

# Default handlers
else:
if mnemonic not in _PIT.OPCODES:
Expand Down Expand Up @@ -546,7 +555,7 @@ def _handle_invoke(classloader, classes, instruction, verbose,

# This is used for special field handling by entitymetadata.
return _PIT._lambda_operations(
classloader, classes, instruction, verbose, obj, arguments
classloader, classes, instruction.pos, verbose, obj, arguments
)
else:
if desc.returns.name != "void":
Expand Down Expand Up @@ -664,8 +673,8 @@ def _handle_1_arg_buffer_call(classloader, classes, instruction, verbose,
def _handle_2_arg_buffer_call(classloader, classes, instruction, verbose,
cls, name, desc, instance, args):
if desc.args[0].name == "java/lang/String" and desc.args[1].name == "int":
max_length = args[1] # not using this at this time
return [Operation(instruction.pos, "write", type="string", field=args[0])]
max_length = int(args[1].value, 0) # the 0 makes it handle the 0x prefix if it's there
return [Operation(instruction.pos, "write", type="string", field=args[0], length=max_length)]
elif desc.args[0].name == "com/mojang/serialization/Codec":
codec = args[0]
value = args[1]
Expand Down Expand Up @@ -702,7 +711,7 @@ def _handle_2_arg_buffer_call(classloader, classes, instruction, verbose,
type=info.method_desc.args[-1].name.replace("/", "."),
var="itv", value="it.next()"))
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose,
classloader, classes, instruction.pos, verbose,
info, [instance, "itv"]
)
# Jank: the part of the program that converts loop+endloop
Expand All @@ -729,7 +738,7 @@ def _handle_2_arg_buffer_call(classloader, classes, instruction, verbose,
info = args[1]
assert isinstance(info, InvokeDynamicInfo)
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose,
classloader, classes, instruction.pos, verbose,
info, [instance, field.value + ".get()"]
)
# Jank: the part of the program that converts loop+endloop
Expand All @@ -754,13 +763,22 @@ def _handle_2_arg_buffer_call(classloader, classes, instruction, verbose,
info = args[1]
assert isinstance(info, InvokeDynamicInfo)
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose,
classloader, classes, instruction.pos, verbose,
info, [instance, field.value]
)
operations.append(Operation(instruction.pos + 1 - SUB_INS_EPSILON, "endif"))
return operations
elif desc.args[0].name == classes.get("idmap"):
return [Operation(instruction.pos, "write", type="varint", field="%s.getId(%s)" % (args[0], args[1]))]
elif desc.args[0].name == "java/util/BitSet":
max_length = int(args[1].value, 0) # the 0 makes it handle the 0x prefix if it's there
return [Operation(instruction.pos, "write", type="bitset", field=args[0], length=max_length)]
elif desc.args[0].name == "java/util/EnumSet":
# bitset with a max length of the enum's constant count
enum_class = classloader[args[1]]
enum_constants = get_enum_constants(enum_class, verbose)
max_length = len(enum_constants)
return [Operation(instruction.pos, "write", type="bitset", field=args[0], length=max_length)]
else:
raise Exception("Unexpected descriptor " + desc.descriptor)

Expand Down Expand Up @@ -794,19 +812,44 @@ def _handle_3_arg_buffer_call(classloader, classes, instruction, verbose,
type="Map.Entry<" + key_type + ", " + val_type + ">",
var="itv", value="it.next()"))
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose,
classloader, classes, instruction.pos, verbose,
key_info, [instance, "itv.getKey()"]
)
# TODO: Does the SUB_INS_EPSILON work correctly here?
# I think this will lead to [1.01, 1.02, 1.03, 1.01, 1.02, 1.03]
# which would get sorted wrongly, but I'm not sure
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose,
classloader, classes, instruction.pos, verbose,
val_info, [instance, "itv.getValue()"]
)
# Same jank as with the one in _handle_2_arg_buffer_call
operations.append(Operation(instruction.pos + 1 - SUB_INS_EPSILON, "endloop"))
return operations
elif desc.args[0].name == "com/mojang/datafixers/util/Either":
# Write a boolean indicating whether it's left, and then call the correct consumer
# with the packetbuffer and value.
operations = []
field = args[0]
left_consumer = args[1]
right_consumer = args[2]
assert isinstance(field, StackOperand)
assert isinstance(left_consumer, InvokeDynamicInfo)
assert isinstance(right_consumer, InvokeDynamicInfo)
operations.append(Operation(instruction.pos, "write", type="boolean",
field=field.value + ".isLeft()"))
operations.append(Operation(instruction.pos, "if",
condition=field.value + ".isLeft()"))
operations += _PIT._lambda_operations(
classloader, classes, instruction.pos, verbose,
left_consumer, [instance, field.value + ".left()"]
)
operations.append(Operation(instruction.pos + 1 - SUB_INS_EPSILON, "else"))
operations += _PIT._lambda_operations(
classloader, classes, instruction.pos + 1, verbose,
right_consumer, [instance, field.value + ".right()"]
)
operations.append(Operation(instruction.pos + 2 - SUB_INS_EPSILON, "endif"))
Comment on lines +840 to +851
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure this will work correctly. There's a different implementation for java/util/Map (which I'm also not sure is correct, but it does guarantee that instruction.pos <= pos and pos < instruction.pos + 1). Though, I also don't 100% remember what pos corresponds to - if it's a byte offset, this might be better as call instructions are multiple bytes.

Copy link
Author

@mat-1 mat-1 Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I didn't add to the instruction pos the else was empty and the instructions were getting mixed with the if instructions, so I think the implementation for Map might actually be wrong. I also just checked by doing print(instruction.pos) at the beginning of a loop where a method is disassembled, and it does look like they're byte offsets since the positions usually increase by more than 1 at a time.

Ok also you don't need to answer this, but why do operations even need to store their position? Can't that just be figured out from the order in the operations list?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, it's probably fine.

Ok also you don't need to answer this, but why do operations even need to store their position? Can't that just be figured out from the order in the operations list?

I'm pretty sure that specific part of the code predates me. I do know that they aren't initially sorted (which is what ordered_operations does). I assume that it's related to endif, which I think gets put into the list as soon as a jump is seen. Not that the if handling is great; see #2 and #3. I think it also relates to converting from a list of instructions into a nested tree structure (see format).

return operations
else:
raise Exception("Unexpected descriptor " + desc.descriptor)

Expand All @@ -826,7 +869,7 @@ def _handle_foreach(classloader, classes, instruction, verbose,
type=consumer.method_desc.args[-1].name.replace("/", "."),
var="itv", value="it.next()"))
operations += _PIT._lambda_operations(
classloader, classes, instruction, verbose, consumer, ["itv"]
classloader, classes, instruction.pos, verbose, consumer, ["itv"]
)
# See comment in _handle_1_arg_buffer_call
operations.append(Operation(instruction.pos + 1 - SUB_INS_EPSILON, "endloop"))
Expand Down Expand Up @@ -914,7 +957,7 @@ def _sub_operations(classloader, classes, instruction, verbose, invoked_class,
return operations

@staticmethod
def _lambda_operations(classloader, classes, instruction, verbose, info, args):
def _lambda_operations(classloader, classes, instruction_pos, verbose, info, args):
assert isinstance(info, InvokeDynamicInfo)
assert len(args) == len(info.instantiated_desc.args)

Expand Down Expand Up @@ -942,7 +985,7 @@ def _lambda_operations(classloader, classes, instruction, verbose, info, args):
for operation in _PIT.ordered_operations(operations):
position += SUB_INS_EPSILON
assert(position < 1)
operation.position = instruction.pos + (position)
operation.position = instruction_pos + (position)

return operations

Expand Down