Skip to content

WIP: Indirect calls on null trapping #961

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

Closed
wants to merge 1 commit into from
Closed

WIP: Indirect calls on null trapping #961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ jobs:
run: npm run clean
- name: Test experimental features
env:
ASC_FEATURES: mutable-globals,threads,reference-types,bigint-integration
ASC_FEATURES: mutable-globals,threads,reference-types,bigint-integration,exception-handling
run: |
npm run test:compiler rt/flags features/js-bigint-integration features/reference-types features/threads
npm run test:compiler rt/flags features/js-bigint-integration features/reference-types features/threads features/exception-handling
test-runtime:
name: "Test runtimes on node: node"
runs-on: ubuntu-latest
Expand Down
17 changes: 9 additions & 8 deletions examples/game-of-life/build/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
global.get $assembly/config/BGR_DEAD
i32.const 16777215
i32.and
else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the automated update feature and it did this. I'm guessing perhaps these files were generated by manual copy/paste or by hand previously maybe?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the output is from an earlier version of Binaryen. It used to emit such whitespace and I guess the example fixtures haven't been updated since then. Most likely nothing to worry about :)

else
global.get $assembly/config/BGR_ALIVE
i32.const -16777216
i32.or
Expand Down Expand Up @@ -134,7 +134,7 @@
i32.eq
if (result i32)
local.get $0
else
else
local.get $2
i32.const 1
i32.sub
Expand All @@ -145,7 +145,7 @@
i32.eq
if (result i32)
i32.const 0
else
else
local.get $2
i32.const 1
i32.add
Expand All @@ -165,7 +165,7 @@
i32.eq
if (result i32)
local.get $1
else
else
local.get $5
i32.const 1
i32.sub
Expand All @@ -176,7 +176,7 @@
i32.eq
if (result i32)
i32.const 0
else
else
local.get $5
i32.const 1
i32.add
Expand Down Expand Up @@ -367,7 +367,7 @@
i32.shl
local.get $14
i32.store
else
else
local.get $5
local.set $16
local.get $2
Expand All @@ -388,7 +388,7 @@
local.get $14
i32.store
end
else
else
local.get $9
i32.const 3
i32.eq
Expand All @@ -412,7 +412,7 @@
i32.shl
local.get $8
i32.store
else
else
local.get $5
local.set $15
local.get $2
Expand Down Expand Up @@ -563,5 +563,6 @@
end
)
(func $null (; 4 ;) (type $FUNCSIG$v)
unreachable
)
)
1 change: 1 addition & 0 deletions examples/mandelbrot/build/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,5 +251,6 @@
end
)
(func $null (; 3 ;) (type $FUNCSIG$v)
unreachable
)
)
1 change: 1 addition & 0 deletions examples/n-body/build/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2225,5 +2225,6 @@
global.set $~lib/rt/stub/offset
)
(func $null (; 24 ;) (type $FUNCSIG$v)
unreachable
)
)
1 change: 1 addition & 0 deletions examples/pson/build/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,5 +484,6 @@
end
)
(func $null (; 18 ;) (type $FUNCSIG$v)
unreachable
)
)
1 change: 1 addition & 0 deletions lib/i64/build/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,5 +749,6 @@
global.set $assembly/i64/hi
)
(func $null (; 31 ;) (type $FUNCSIG$v)
unreachable
)
)
40 changes: 35 additions & 5 deletions src/compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,11 @@ export class Compiler extends DiagnosticEmitter {
module.removeGlobal(BuiltinSymbols.rtti_base);
if (this.runtimeFeatures & RuntimeFeatures.RTTI) compileRTTI(this);

// set up function table
var functionTable = this.functionTable;
module.setFunctionTable(functionTable.length, 0xffffffff, functionTable, module.i32(0));
this.addNullPointerExceptionHandling(module);

// update the heap base pointer
var memoryOffset = this.memoryOffset;
memoryOffset = i64_align(memoryOffset, options.usizeType.byteSize);
Expand Down Expand Up @@ -447,11 +452,6 @@ export class Compiler extends DiagnosticEmitter {
// import memory if requested (default memory is named '0' by Binaryen)
if (options.importMemory) module.addMemoryImport("0", "env", "memory", isSharedMemory);

// set up function table
var functionTable = this.functionTable;
module.setFunctionTable(functionTable.length, 0xffffffff, functionTable, module.i32(0));
module.addFunction("null", this.ensureFunctionType(null, Type.void), null, module.block(null, []));

// import table if requested (default table is named '0' by Binaryen)
if (options.importTable) module.addTableImport("0", "env", "table");

Expand All @@ -462,6 +462,36 @@ export class Compiler extends DiagnosticEmitter {
return module;
}

addNullPointerExceptionHandling(module: Module) {
if (this.options.hasFeature(Feature.EXCEPTION_HANDLING)) {
let message = this.ensureStaticString("null is not a function");
Copy link
Contributor Author

@jayphelps jayphelps Nov 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This returns an index that is emitted but for some reason the string isn't interned in a data section AFAICT. Anyone see what I did wrong?

Also, bikeshedding welcome on the error message, as always.

let exceptionType = module.addFunctionType(
"NullPointerException",
NativeType.None,
[NativeType.I32]
);

module.addEvent("NullPointerException", 0, exceptionType);
Copy link
Contributor Author

@jayphelps jayphelps Nov 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

For that real JavaScript feeling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw this string is only used as a .wat debugging symbol it isn't used at runtime. Wasm exceptions don't (currently) have names like JS exceptions do. They're all WebAssembly.Runtime instances with a message field of just wasm exception from the JS side of things.

module.addFunction(
"null",
this.ensureFunctionType(null, Type.void),
null,
module.block(null, [
module.throw("NullPointerException", [message])
])
);
} else {
module.addFunction(
"null",
this.ensureFunctionType(null, Type.void),
null,
module.block(null, [
module.unreachable()
])
);
}
}

// === Exports ==================================================================================

/** Applies the respective module exports for the specified file. */
Expand Down
17 changes: 9 additions & 8 deletions tests/allocators/buddy/untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
local.get $1
i32.const 3
i32.and
else
else
i32.const 0
end
if
Expand Down Expand Up @@ -1293,7 +1293,7 @@
i32.le_u
if (result i32)
i32.const 1
else
else
local.get $5
local.get $3
i32.add
Expand Down Expand Up @@ -1413,7 +1413,7 @@
end
end
end
else
else
local.get $4
i32.const 7
i32.and
Expand Down Expand Up @@ -1554,7 +1554,7 @@
local.get $4
i32.load8_u
i32.eq
else
else
i32.const 0
end
if
Expand All @@ -1581,7 +1581,7 @@
local.get $4
i32.load8_u
i32.sub
else
else
i32.const 0
end
end
Expand Down Expand Up @@ -2019,7 +2019,7 @@
i32.ne
if (result i32)
i32.const 1
else
else
local.get $2
i32.const 0
i32.eq
Expand Down Expand Up @@ -2060,7 +2060,7 @@
i32.div_u
global.get $assembly/buddy/List.SIZE
i32.add
else
else
local.get $3
end
local.set $4
Expand Down Expand Up @@ -2164,7 +2164,7 @@
call $assembly/buddy/parent_is_split
if (result i32)
i32.const 1
else
else
local.get $1
global.get $assembly/buddy/bucket_limit
i32.eq
Expand Down Expand Up @@ -2207,5 +2207,6 @@
call $start:assembly/index
)
(func $null (; 26 ;) (type $FUNCSIG$v)
unreachable
)
)
Loading