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

Inconsistent record/replay #3

Closed
aoli-al opened this issue Jan 21, 2024 · 9 comments
Closed

Inconsistent record/replay #3

aoli-al opened this issue Jan 21, 2024 · 9 comments
Assignees
@doehyunbaek

Comments

@aoli-al
Copy link
Contributor

aoli-al commented Jan 21, 2024 •
edited
Loading

Hi developers,

Great tool! I've tried your tool with following code but got inconsistent traces.

use wasm_bindgen::prelude::*;



#[wasm_bindgen(module = "/name.js")]
extern "C" {
    fn name(src: String) -> String;
}

#[wasm_bindgen]
extern "C" {
    // Use `js_namespace` here to bind `console.log(..)` instead of just
    // `log(..)`
    #[wasm_bindgen(js_namespace = console)]
    fn log(s: &str);

    // The `console.log` is quite polymorphic, so we can bind it with multiple
    // signatures. Note that we need to use `js_name` to ensure we always call
    // `log` in JS.
    #[wasm_bindgen(js_namespace = console, js_name = log)]
    fn log_u32(a: u32);

    // Multiple arguments too!
    #[wasm_bindgen(js_namespace = console, js_name = log)]
    fn log_many(a: &str, b: &str);
}


 #[wasm_bindgen]
pub fn greet(a: &str) -> String {
    let res = format!("Hello, {}!", name(a.to_string()));
    log(&res);
    res
}
import init, { create, process, math, greet } from './pkg/demo.js';

async function run() {
    await init();

    console.log(greet("Hello"));
    console.log(greet("World"));
}


run();

I got following traces:

[Expected]
IF;0;wbg;__wbg_name_3abd14ecb7c22373
IF;1;wbg;__wbg_log_24dc0bd9d833d95d
EC;45;__wbindgen_add_to_stack_pointer;-16
ER
EC;24;__wbindgen_malloc;5,1
ER
EC;11;greet;1048560,1114120,5
L;0;memory;1114120;72
L;0;memory;1114121;101
L;0;memory;1114122;108
L;0;memory;1114123;108
L;0;memory;1114124;111
IC;0;__wbg_name_3abd14ecb7c22373
EC;24;__wbindgen_malloc;9,1
EC;33;__wbindgen_free;1114136,5,1
IR;0;__wbg_name_3abd14ecb7c22373;
L;0;memory;1048500;9
L;0;memory;1048496;40
L;0;memory;1048498;17
L;0;memory;1114152;72
L;0;memory;1114153;101
L;0;memory;1114154;108
L;0;memory;1114155;108
L;0;memory;1114156;111
L;0;memory;1114157;82
L;0;memory;1114158;117
L;0;memory;1114159;115
L;0;memory;1114160;116
IC;1;__wbg_log_24dc0bd9d833d95d
IR;1;__wbg_log_24dc0bd9d833d95d;
ER
EC;45;__wbindgen_add_to_stack_pointer;16
ER
EC;33;__wbindgen_free;1114168,17,1
ER
EC;45;__wbindgen_add_to_stack_pointer;-16
ER
EC;24;__wbindgen_malloc;5,1
ER
EC;11;greet;1048560,1114120,5
L;0;memory;1114120;87
L;0;memory;1114121;111
L;0;memory;1114122;114
L;0;memory;1114123;108
L;0;memory;1114124;100
IC;0;__wbg_name_3abd14ecb7c22373
EC;24;__wbindgen_malloc;9,1
EC;33;__wbindgen_free;1114136,5,1
IR;0;__wbg_name_3abd14ecb7c22373;
L;0;memory;1114152;87
L;0;memory;1114153;111
L;0;memory;1114154;114
L;0;memory;1114156;100
L;0;memory;1114160;116
IC;1;__wbg_log_24dc0bd9d833d95d
IR;1;__wbg_log_24dc0bd9d833d95d;
ER
EC;45;__wbindgen_add_to_stack_pointer;16
ER
EC;33;__wbindgen_free;1114168,17,1
ER

[Actual]
IF;0;wbg;__wbg_name_3abd14ecb7c22373
IF;1;wbg;__wbg_log_24dc0bd9d833d95d
EC;45;__wbindgen_add_to_stack_pointer;-16
ER
EC;24;__wbindgen_malloc;5,1
ER
EC;11;greet;1048560,1114120,5
L;0;memory;1114120;72
L;0;memory;1114121;101
L;0;memory;1114122;108
L;0;memory;1114123;108
L;0;memory;1114124;111
IC;0;__wbg_name_3abd14ecb7c22373
EC;24;__wbindgen_malloc;9,1
EC;33;__wbindgen_free;1114136,5,1
IR;0;__wbg_name_3abd14ecb7c22373;
L;0;memory;1048500;9
L;0;memory;1048496;40
L;0;memory;1048498;17
L;0;memory;1114152;72
L;0;memory;1114153;101
L;0;memory;1114154;108
L;0;memory;1114155;108
L;0;memory;1114156;111
L;0;memory;1114157;82
L;0;memory;1114158;117
L;0;memory;1114159;115
L;0;memory;1114160;116
IC;1;__wbg_log_24dc0bd9d833d95d
IR;1;__wbg_log_24dc0bd9d833d95d;
ER
EC;45;__wbindgen_add_to_stack_pointer;16
ER
EC;33;__wbindgen_free;1114168,17,1
ER
EC;45;__wbindgen_add_to_stack_pointer;-16
ER
EC;24;__wbindgen_malloc;5,1
ER
EC;11;greet;1048560,1114120,5
IC;0;__wbg_name_3abd14ecb7c22373
EC;24;__wbindgen_malloc;9,1
EC;33;__wbindgen_free;1114136,5,1
IR;0;__wbg_name_3abd14ecb7c22373;
L;0;memory;1114152;87
L;0;memory;1114153;111
L;0;memory;1114154;114
L;0;memory;1114156;100
L;0;memory;1114160;116
IC;1;__wbg_log_24dc0bd9d833d95d
IR;1;__wbg_log_24dc0bd9d833d95d;
ER
EC;45;__wbindgen_add_to_stack_pointer;16
ER
EC;33;__wbindgen_free;1114168,17,1
ER

If seems that the generator put the second memory load world in the wrong location.

import fs from 'fs'
import path from 'path'
let instance
let imports = {}
imports['wbg'] = {}
let global_0 = -1
imports['wbg']['__wbg_name_3abd14ecb7c22373'] = () => {
global_0++
switch (global_0) {
case 0:
instance.exports.__wbindgen_malloc(9,1)
instance.exports.__wbindgen_free(1114136,5,1)
new Uint8Array(instance.exports.memory.buffer)[1048500] = 9
new Uint8Array(instance.exports.memory.buffer)[1048496] = 40
new Uint8Array(instance.exports.memory.buffer)[1048498] = 17
new Uint8Array(instance.exports.memory.buffer)[1114152] = 72
new Uint8Array(instance.exports.memory.buffer)[1114153] = 101
new Uint8Array(instance.exports.memory.buffer)[1114154] = 108
new Uint8Array(instance.exports.memory.buffer)[1114155] = 108
new Uint8Array(instance.exports.memory.buffer)[1114156] = 111
new Uint8Array(instance.exports.memory.buffer)[1114157] = 82
new Uint8Array(instance.exports.memory.buffer)[1114158] = 117
new Uint8Array(instance.exports.memory.buffer)[1114159] = 115
new Uint8Array(instance.exports.memory.buffer)[1114160] = 116
break
case 1:
instance.exports.__wbindgen_malloc(9,1)
instance.exports.__wbindgen_free(1114136,5,1)
new Uint8Array(instance.exports.memory.buffer)[1114152] = 87
new Uint8Array(instance.exports.memory.buffer)[1114153] = 111
new Uint8Array(instance.exports.memory.buffer)[1114154] = 114
new Uint8Array(instance.exports.memory.buffer)[1114156] = 100
new Uint8Array(instance.exports.memory.buffer)[1114160] = 116
break
}
if ((global_0 >= 0) && global_0 < 2) {
return undefined }
}
let global_1 = -1
imports['wbg']['__wbg_log_24dc0bd9d833d95d'] = () => {
global_1++
switch (global_1) {
case 0:
// THIS SECTION IS IN THE WRONG LOCATION
new Uint8Array(instance.exports.memory.buffer)[1114120] = 87
new Uint8Array(instance.exports.memory.buffer)[1114121] = 111
new Uint8Array(instance.exports.memory.buffer)[1114122] = 114
new Uint8Array(instance.exports.memory.buffer)[1114123] = 108
new Uint8Array(instance.exports.memory.buffer)[1114124] = 100
// THIS SECTION IS IN THE WRONG LOCATION
break
}
if ((global_1 >= 0) && global_1 < 2) {
return undefined }
}
export function replay(wasm) {instance = wasm.instance
instance.exports.__wbindgen_add_to_stack_pointer(-16)
instance.exports.__wbindgen_malloc(5,1)
new Uint8Array(instance.exports.memory.buffer)[1114120] = 72
new Uint8Array(instance.exports.memory.buffer)[1114121] = 101
new Uint8Array(instance.exports.memory.buffer)[1114122] = 108
new Uint8Array(instance.exports.memory.buffer)[1114123] = 108
new Uint8Array(instance.exports.memory.buffer)[1114124] = 111
instance.exports.greet(1048560,1114120,5)
instance.exports.__wbindgen_add_to_stack_pointer(16)
instance.exports.__wbindgen_free(1114168,17,1)
instance.exports.__wbindgen_add_to_stack_pointer(-16)
instance.exports.__wbindgen_malloc(5,1)
instance.exports.greet(1048560,1114120,5)
instance.exports.__wbindgen_add_to_stack_pointer(16)
instance.exports.__wbindgen_free(1114168,17,1)
}
export function instantiate(wasmBinary) {
return WebAssembly.instantiate(wasmBinary, imports)
}
if (process.argv[2] === 'run') {
const p = path.join(path.dirname(import.meta.url).replace(/^file:/, ''), 'index.wasm')
const wasmBinary = fs.readFileSync(p)
instantiate(wasmBinary).then((wasm) => replay(wasm))
}
@jakobgetz
Copy link
Collaborator

Hi,

thanks for your interest. Even though this repository is currently public there does not yet exist a stable release yet, so bugs like the one you mentioned can occur frequently. We are currently actively developing on this tool and have a few things on our bucket lists. I will keep you updated on the fixing of this bug.

@doehyunbaek
Copy link
Collaborator

Thanks for the report! I will try to look into this this week.

@doehyunbaek
Copy link
Collaborator

Hi @aoli-al, can you provide a repository that contains the code example you provided, making it easier to reproduce the problem? I think I can try to make it somehow work with the code you provided but I think with a runnable code repository it would make a debugging process way simpler.

@doehyunbaek doehyunbaek self-assigned this Jan 22, 2024
@aoli-al aoli-al mentioned this issue Jan 22, 2024
Merged
@aoli-al
Copy link
Contributor Author

aoli-al commented Jan 22, 2024

Yes, I've created #4 to add this test case.

@jakobgetz
Copy link
Collaborator

Thanks for the PR. It would best to reduce this test case to the core issue and then create a minimal node test-case that reproduces the bug. Once we figure out the core issue, we can fix the bug. Offline test cases are intended to test the overall wasm-r3 pipeline not the record and replay logic.

@doehyunbaek
Copy link
Collaborator

doehyunbaek commented Jan 22, 2024 •
edited
Loading

Hmm but I think this case is good enough. I'll take a look at the offline test case and make a node case out of it.

After that we can decide whether we should keep the offline test or not.

@aoli-al
Copy link
Contributor Author

aoli-al commented Jan 22, 2024 •
edited
Loading

Yes, this is the minimum example I can get. The core issue is when you call greet, it calls console.log which is an import call. This will set lastFunctionReturn here.

While calling greet second time, replay will check lastFunctionReturn here and put the following memory load section at in the function return section of console.log.

new Uint8Array(instance.exports.memory.buffer)[1114120] = 87
new Uint8Array(instance.exports.memory.buffer)[1114121] = 111
new Uint8Array(instance.exports.memory.buffer)[1114122] = 114
new Uint8Array(instance.exports.memory.buffer)[1114123] = 108
new Uint8Array(instance.exports.memory.buffer)[1114124] = 100

The correct version of replay function should be:

instance.exports.__wbindgen_add_to_stack_pointer(-16)
instance.exports.__wbindgen_malloc(5,1)
new Uint8Array(instance.exports.memory.buffer)[1114120] = 72
new Uint8Array(instance.exports.memory.buffer)[1114121] = 101
new Uint8Array(instance.exports.memory.buffer)[1114122] = 108
new Uint8Array(instance.exports.memory.buffer)[1114123] = 108
new Uint8Array(instance.exports.memory.buffer)[1114124] = 111
instance.exports.greet(1048560,1114120,5)
instance.exports.__wbindgen_add_to_stack_pointer(16)
instance.exports.__wbindgen_free(1114168,17,1)
instance.exports.__wbindgen_add_to_stack_pointer(-16)
instance.exports.__wbindgen_malloc(5,1)
new Uint8Array(instance.exports.memory.buffer)[1114120] = 87
new Uint8Array(instance.exports.memory.buffer)[1114121] = 111
new Uint8Array(instance.exports.memory.buffer)[1114122] = 114
new Uint8Array(instance.exports.memory.buffer)[1114123] = 108
new Uint8Array(instance.exports.memory.buffer)[1114124] = 100
instance.exports.greet(1048560,1114120,5)
instance.exports.__wbindgen_add_to_stack_pointer(16)
instance.exports.__wbindgen_free(1114168,17,1)

I think the main issue here is how to associate correct bytes to the imported function calls.

@doehyunbaek
Copy link
Collaborator

I think with kind of logic in commit 4f05d7a , this solves this case. Node tests and given case is passing but one offline test and some of the online tests are passing due to some other reasons I think. Will try to solve them after the vacation and write a detailed writeup on this.

@doehyunbaek
Copy link
Collaborator

03fe3fc, which contain the fix of 4f05d7a fixes the issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@doehyunbaek doehyunbaek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@aoli-al @jakobgetz @doehyunbaek