Bug 252538 - [Wasm-GC] Incorrect generated LLInt code for structs containing ref fields
Summary: [Wasm-GC] Incorrect generated LLInt code for structs containing ref fields
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebAssembly (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 247394
  Show dependency treegraph
 
Reported: 2023-02-18 16:31 PST by Tim Chevalier
Modified: 2023-03-20 18:19 PDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Chevalier 2023-02-18 16:31:20 PST
The following test:

```
//@ runWebAssemblySuite("--useWebAssemblyTypedFunctionReferences=true", "--useWebAssemblyGC=true")

import * as assert from "../assert.js";
import { compile, instantiate } from "./wast-wrapper.js";

function module(bytes, valid = true) {
  let buffer = new ArrayBuffer(bytes.length);
  let view = new Uint8Array(buffer);
  for (let i = 0; i < bytes.length; ++i) {
    view[i] = bytes.charCodeAt(i);
  }
  return new WebAssembly.Module(buffer);
}

function testStructDeclaration() {

  let m = instantiate(`
    (module
      (type $a (array i32))
      (type $s (struct (field (ref $a)) (field (ref $a)) (field (ref $a))))

      (func $new (result (ref $s))
         (struct.new_canon $s (array.new_canon_default $a (i32.const 5))
                              (array.new_canon_default $a (i32.const 17))
                              (array.new_canon_default $a (i32.const 0))))

      (func (export "len0") (result i32)
         (struct.get $s 0 (call $new))
         (array.len))
      (func (export "len1") (result i32)
         (struct.get $s 1 (call $new))
         (array.len))
      (func (export "len2") (result i32)
         (struct.get $s 2 (call $new))
         (array.len)))`);

    assert.eq(m.exports.len0(), 5);
    assert.eq(m.exports.len1(), 17);
    assert.eq(m.exports.len2(), 0);
}

testStructDeclaration();
```

fails on the second `assert.eq` call. LLInt generates the following bytecode for the `$new` function:

```
<?>.wasm-function[0] : () -> [Ref]
wasm size: 20 bytes
bytecode: 10 instructions (0 16-bit instructions, 3 32-bit instructions); 85 bytes; 0 parameter(s); 18 local(s); 22 callee register(s)
[   0] enter              
[   1] **array_new        dst:loc18, size:5(const0), value:<invalid>, typeIndex:0, arrayNewKind:1
[  23] **array_new        dst:loc19, size:17(const1), value:<invalid>, typeIndex:0, arrayNewKind:1
[  45] **array_new        dst:loc20, size:0(const2), value:<invalid>, typeIndex:0, arrayNewKind:1
[  67] mov                dst:loc19, src:loc20
[  70] mov                dst:loc20, src:loc19
[  73] mov                dst:loc21, src:loc18
[  76] struct_new         dst:loc18, typeIndex:1, useDefault:false, firstValue:loc21
[  81] mov                dst:loc4, src:loc18
[  84] ret                
```

This assigns loc20 (the third argument to struct.new) into loc19, overwriting the second argument to struct.new. So if we write `(struct.new_canon $s e1 e2 e3)`, the result is the struct `{ e1, e3, e3 }`.
Comment 1 Tim Chevalier 2023-02-18 21:18:33 PST
Pull request: https://github.com/WebKit/WebKit/pull/10342
Comment 2 Radar WebKit Bug Importer 2023-02-25 16:32:20 PST
<rdar://problem/105931873>
Comment 3 EWS 2023-03-20 18:19:37 PDT
Committed 261902@main (400ec97f086f): <https://commits.webkit.org/261902@main>

Reviewed commits have been landed. Closing PR #10342 and removing active labels.