ForgeFlower
ForgeFlower copied to clipboard
MinecraftForge
Casts missing parentheses and causing precision loss
Could it be FernFlower causing this bug? https://github.com/MinecraftForge/MinecraftForge/issues/2321
The (int) cast needs to be applied to the entire p_181663_1_ * 127 expression, but the parentheses are left out so that it only casts p_181663_1_, changing the calculation.
The affected method is BufferBuilder.normal
As decompiled by FF, it looks like this:
public BufferBuilder func_181663_c(float p_181663_1_, float p_181663_2_, float p_181663_3_) {
int i = this.field_178997_d * this.field_179011_q.func_177338_f() + this.field_179011_q.func_181720_d(this.field_181678_g);
switch(this.field_181677_f.func_177367_b()) {
case FLOAT:
this.field_179001_a.putFloat(i, p_181663_1_);
this.field_179001_a.putFloat(i + 4, p_181663_2_);
this.field_179001_a.putFloat(i + 8, p_181663_3_);
break;
case UINT:
case INT:
this.field_179001_a.putInt(i, (int)p_181663_1_);
this.field_179001_a.putInt(i + 4, (int)p_181663_2_);
this.field_179001_a.putInt(i + 8, (int)p_181663_3_);
break;
case USHORT:
case SHORT:
this.field_179001_a.putShort(i, (short)((int)p_181663_1_ * 32767 & '\uffff'));
this.field_179001_a.putShort(i + 2, (short)((int)p_181663_2_ * 32767 & '\uffff'));
this.field_179001_a.putShort(i + 4, (short)((int)p_181663_3_ * 32767 & '\uffff'));
break;
case UBYTE:
case BYTE:
this.field_179001_a.put(i, (byte)((int)p_181663_1_ * 127 & 255));
this.field_179001_a.put(i + 1, (byte)((int)p_181663_2_ * 127 & 255));
this.field_179001_a.put(i + 2, (byte)((int)p_181663_3_ * 127 & 255));
}
this.func_181667_k();
return this;
}
As disassembled by javap, it looks like this:
public net.minecraft.client.renderer.BufferBuilder func_181663_c(float, float, float);
descriptor: (FFF)Lnet/minecraft/client/renderer/BufferBuilder;
flags: ACC_PUBLIC
Code:
stack=4, locals=5, args_size=4
0: aload_0
1: getfield #99 // Field field_178997_d:I
4: aload_0
5: getfield #101 // Field field_179011_q:Lnet/minecraft/client/renderer/vertex/VertexFormat;
8: invokevirtual #106 // Method net/minecraft/client/renderer/vertex/VertexFormat.func_177338_f:()I
11: imul
12: aload_0
13: getfield #101 // Field field_179011_q:Lnet/minecraft/client/renderer/vertex/VertexFormat;
16: aload_0
17: getfield #300 // Field field_181678_g:I
20: invokevirtual #329 // Method net/minecraft/client/renderer/vertex/VertexFormat.func_181720_d:(I)I
23: iadd
24: istore 4
26: getstatic #332 // Field net/minecraft/client/renderer/BufferBuilder$1.field_210254_a:[I
29: aload_0
30: getfield #298 // Field field_181677_f:Lnet/minecraft/client/renderer/vertex/VertexFormatElement;
33: invokevirtual #336 // Method net/minecraft/client/renderer/vertex/VertexFormatElement.func_177367_b:()Lnet/minecraft/client/renderer/vertex/VertexFormatElement$EnumType;
36: invokevirtual #339 // Method net/minecraft/client/renderer/vertex/VertexFormatElement$EnumType.ordinal:()I
39: iaload
40: tableswitch { // 1 to 7
1: 84
2: 125
3: 125
4: 169
5: 169
6: 239
7: 239
default: 303
}
84: aload_0
85: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
88: iload 4
90: fload_1
91: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer;
94: pop
95: aload_0
96: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
99: iload 4
101: iconst_4
102: iadd
103: fload_2
104: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer;
107: pop
108: aload_0
109: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
112: iload 4
114: bipush 8
116: iadd
117: fload_3
118: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer;
121: pop
122: goto 303
125: aload_0
126: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
129: iload 4
131: fload_1
132: f2i
133: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer;
136: pop
137: aload_0
138: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
141: iload 4
143: iconst_4
144: iadd
145: fload_2
146: f2i
147: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer;
150: pop
151: aload_0
152: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
155: iload 4
157: bipush 8
159: iadd
160: fload_3
161: f2i
162: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer;
165: pop
166: goto 303
169: aload_0
170: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
173: iload 4
175: fload_1
176: f2i
177: sipush 32767
180: imul
181: ldc_w #484 // int 65535
184: iand
185: i2s
186: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer;
189: pop
190: aload_0
191: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
194: iload 4
196: iconst_2
197: iadd
198: fload_2
199: f2i
200: sipush 32767
203: imul
204: ldc_w #484 // int 65535
207: iand
208: i2s
209: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer;
212: pop
213: aload_0
214: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
217: iload 4
219: iconst_4
220: iadd
221: fload_3
222: f2i
223: sipush 32767
226: imul
227: ldc_w #484 // int 65535
230: iand
231: i2s
232: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer;
235: pop
236: goto 303
239: aload_0
240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
243: iload 4
245: fload_1
246: f2i
247: bipush 127
249: imul
250: sipush 255
253: iand
254: i2b
255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
258: pop
259: aload_0
260: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
263: iload 4
265: iconst_1
266: iadd
267: fload_2
268: f2i
269: bipush 127
271: imul
272: sipush 255
275: iand
276: i2b
277: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
280: pop
281: aload_0
282: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
285: iload 4
287: iconst_2
288: iadd
289: fload_3
290: f2i
291: bipush 127
293: imul
294: sipush 255
297: iand
298: i2b
299: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
302: pop
303: aload_0
304: invokespecial #357 // Method func_181667_k:()V
307: aload_0
308: areturn
StackMapTable: number_of_entries = 5
frame_type = 252 /* append */
offset_delta = 84
locals = [ int ]
frame_type = 40 /* same */
frame_type = 43 /* same */
frame_type = 251 /* same_frame_extended */
offset_delta = 69
frame_type = 255 /* full_frame */
offset_delta = 63
locals = [ class net/minecraft/client/renderer/BufferBuilder ]
stack = []
LineNumberTable:
line 464: 0
line 465: 26
line 467: 84
line 468: 95
line 469: 108
line 470: 122
line 473: 125
line 474: 137
line 475: 151
line 476: 166
line 479: 169
line 480: 190
line 481: 213
line 482: 236
line 485: 239
line 486: 259
line 487: 281
line 490: 303
line 491: 307
LocalVariableTable:
Start Length Slot Name Signature
0 309 0 this Lnet/minecraft/client/renderer/BufferBuilder;
0 309 1 p_181663_1_ F
0 309 2 p_181663_2_ F
0 309 3 p_181663_3_ F
26 283 4 lvt_4_1_ I
Since this is a lot of text, I'll focus just on just the BYTE and UBYTE path.
Here's the same disassembly, trimmed:
public net.minecraft.client.renderer.BufferBuilder func_181663_c(float, float, float);
Code:
// Computing i - snip
// switch
40: tableswitch { // 1 to 7
// snip cases we don't care about
6: 239 // for UBYTE jump to 239
7: 239 // for BYTE jump to 239
default: 303
}
// snip code for cases we don't care about
239: aload_0
240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
243: iload 4
245: fload_1
246: f2i
247: bipush 127
249: imul
250: sipush 255
253: iand
254: i2b
255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
258: pop
259: aload_0
260: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
263: iload 4
265: iconst_1
266: iadd
267: fload_2
268: f2i
269: bipush 127
271: imul
272: sipush 255
275: iand
276: i2b
277: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
280: pop
281: aload_0
282: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
285: iload 4
287: iconst_2
288: iadd
289: fload_3
290: f2i
291: bipush 127
293: imul
294: sipush 255
297: iand
298: i2b
299: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
302: pop
// Exiting the method
303: aload_0
304: invokespecial #357 // Method func_181667_k:()V
307: aload_0
308: areturn
And now to dissect a tiny section of that specifically -- one call to put:
; Put `this` onto the stack
; Stack: [this]
239: aload_0
; Remove `this` from the stack and put `this.byteBuffer` onto it
; Stack: [this.byteBuffer]
240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
; Load local variable 4 (which is i, or alternatively lvt_4_1_), and put it onto the stack
; Stack: [this.byteBuffer, i]
243: iload 4
; Load local variable 1 (which is x, or alternatively p_181663_1_), and put it onto the stack.
; Stack: [this.byteBuffer, i, x]
245: fload_1
; Convert the top of the stack from a float to an int and put it into the stack
; Stack: [this.byteBuffer, i, ((int)x)]
246: f2i
; Put the number 127 onto the stack
; Stack: [this.byteBuffer, i, ((int)x), 127]
247: bipush 127
; Take the top two values from the stack and perform an integer multiplication
; and then put that onto the stack
; Stack: [this.byteBuffer, i, (((int)x) * 127)]
249: imul
; Take the number 255 and put it onto the stack
; Stack: [this.byteBuffer, i, (((int)x) * 127), 255]
250: sipush 255
; And the two top values together and put the result onto the stack
; Stack: [this.byteBuffer, i, ((((int)x) * 127) & 255)]
253: iand
; Convert the top of the stack from an int into a byte and put it onto the stack
; Stack: [this.byteBuffer, i, ((byte)((((int)x) * 127) & 255))]
254: i2b
; Call `this.byteBuffer.put`. Two arguments are popped off
; and then the object to call it on.
; The resultant call is `this.byteBuffer.put(i, ((byte)((((int)x) * 127) & 255)))`
; `ByteBuffer.put` puts the result back onto the stack, so:
; Stack: [this.byteBuffer]
255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
; Discard the return value of put
; Stack: []
280: pop
This shows that the cast in the bytecode happens before the multiplication, and the original code actually does have it. So it's a mistake on Mojang's end, not a bug with FernFlower.
I'm mostly ignorant on the actual rendering code, so I need to ask: how would this manifest itself ingame? Is there any way that it could empirically be confirmed that this is a vanilla issue?
There's an image here: https://github.com/MinecraftForge/MinecraftForge/pull/2437