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.

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

Panic when running twiggy on multithreaded wasm module #326

Closed
alexcrichton opened this issue Jul 19, 2019 · 14 comments · Fixed by #744 · May be fixed by #477
Closed

Panic when running twiggy on multithreaded wasm module #326

alexcrichton opened this issue Jul 19, 2019 · 14 comments · Fixed by #744 · May be fixed by #477
Labels
bug Something isn't working

Comments

@alexcrichton
Copy link
Contributor

I ran into an assertion when running twiggy top on a multithreaded wasm module (attached here)

$ twiggy top raytrace_parallel_bg.wasm
thread 'main' panicked at 'should not parse the same key into multiple items', /home/alex/.cargo/registry/src/github.com-1ecc6299db9ec823/twiggy-ir-0.6.0/./ir.rs:53:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

raytrace_parallel_bg.wasm.gz

@data-pup
Copy link
Member

Thank you for filing an issue! I will start taking a look into the cause of this.

@data-pup
Copy link
Member

data-pup commented Sep 9, 2019

Just wanted to follow up and say sorry that this has taken a while to get to, I've been a little caught up with life things but this is at the top of my docket 🙂

sepiropht added a commit to sepiropht/twiggy that referenced this issue Oct 25, 2019
@fitzgen
Copy link
Member

fitzgen commented Nov 11, 2019

Here's a smaller test case, could likely make it even smaller with wasm-reduce or creduce. I suspect that just including passive data segments or something like that is enough.

(module
 (type $FUNCSIG$i (func (result i32)))
 (type $FUNCSIG$vi (func (param i32)))
 (type $FUNCSIG$vj (func (param i64)))
 (type $FUNCSIG$vf (func (param f32)))
 (type $FUNCSIG$vd (func (param f64)))
 (type $FUNCSIG$fifdddfVi (func (param i32 f32 f64 f64 f64 f32 v128 i32) (result f32)))
 (import "fuzzing-support" "log-i32" (func $log-i32 (param i32)))
 (import "fuzzing-support" "log-i64" (func $log-i64 (param i64)))
 (import "fuzzing-support" "log-f32" (func $log-f32 (param f32)))
 (import "fuzzing-support" "log-f64" (func $log-f64 (param f64)))
 (memory $0 1 1)
 (data (i32.const 0) "\1c\03a;")
 (table $0 1 funcref)
 (elem (i32.const 0) $func_6)
 (global $global$0 (mut i32) (i32.const 2))
 (global $global$1 (mut i32) (i32.const -268435456))
 (global $global$2 (mut i64) (i64.const 1024))
 (global $global$3 (mut i64) (i64.const -87))
 (global $global$4 (mut f32) (f32.const 18231))
 (global $hangLimit (mut i32) (i32.const 10))
 (export "hashMemory" (func $hashMemory))
 (export "func_6" (func $func_6))
 (export "hangLimitInitializer" (func $hangLimitInitializer))
 (func $hashMemory (; 4 ;) (type $FUNCSIG$i) (result i32)
  (local $0 i32)
  (local.set $0
   (i32.const 5381)
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=1
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=2
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=3
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=4
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=5
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=6
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=7
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=8
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=9
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=10
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=11
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=12
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=13
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=14
     (i32.const 0)
    )
   )
  )
  (local.set $0
   (i32.xor
    (i32.add
     (i32.shl
      (local.get $0)
      (i32.const 5)
     )
     (local.get $0)
    )
    (i32.load8_u offset=15
     (i32.const 0)
    )
   )
  )
  (local.get $0)
 )
 (func $func_5 (; 5 ;) (param $0 i32)
  (block
   (if
    (i32.eqz
     (global.get $hangLimit)
    )
    (return)
   )
   (global.set $hangLimit
    (i32.sub
     (global.get $hangLimit)
     (i32.const 1)
    )
   )
  )
  (data.drop 0)
 )
 (func $func_6 (; 6 ;) (type $FUNCSIG$fifdddfVi) (param $0 i32) (param $1 f32) (param $2 f64) (param $3 f64) (param $4 f64) (param $5 f32) (param $6 v128) (param $7 i32) (result f32)
  (block
   (if
    (i32.eqz
     (global.get $hangLimit)
    )
    (return
     (local.get $5)
    )
   )
   (global.set $hangLimit
    (i32.sub
     (global.get $hangLimit)
     (i32.const 1)
    )
   )
  )
  (block $label$0 (result f32)
   (local.get $5)
  )
 )
 (func $hangLimitInitializer (; 7 ;)
  (global.set $hangLimit
   (i32.const 10)
  )
 )
)

@data-pup
Copy link
Member

Thanks so much @fitzgen ❤️

@d3lm
Copy link

d3lm commented Jun 7, 2020

Running into the same issue. Trying to inspect a wasm module generated by wasm-pack to find out more about the binary size.

Any updates?

@data-pup
Copy link
Member

Hey @d3lm, I'm sorry about that! I've opened up a draft PR, working on it.

@d3lm
Copy link

d3lm commented Jun 26, 2020

Very cool! Thanks 🙏

@AlexEne AlexEne added the bug Something isn't working label Aug 6, 2021
@maxammann
Copy link

I just hit the same problem, I also noticed that a binary without atomic support is smaller.

@michelledaviest
Copy link

You can trigger the same bug with the following code example as well:

(module
    (func $main 
        i32.const 42
        drop
    )    
    (start $main)
)

@AlexEne
Copy link
Collaborator

AlexEne commented Aug 18, 2022

Thanks for this example code. If I get some time these weeks I'll check if the PR above can be fixed-up and merged!

@RReverser
Copy link
Member

Hey @d3lm, I'm sorry about that! I've opened up a draft PR, working on it.

I'm guessing not, but have you had any luck?

@d3lm
Copy link

d3lm commented Nov 7, 2022

@RReverser It's been so long. I think I may need to check it again, tho I am seeing that the bug is still present and there's also a minimal reproducable.

@erwanvivien
Copy link

Hey, is there any update to this ?
If not, where could I look into it?

@gregtatum
Copy link

If anyone wants a work around:

  • Clone the repo
  • Comment out the assertion
  • Run cargo run --bin twiggy -- top path/to/file.wasm

I don't know if the results are still correct, but I'm at least getting some output to play around with.

diff --git a/ir/ir.rs b/ir/ir.rs
index f36af5d..4d9930e 100644
--- a/ir/ir.rs
+++ b/ir/ir.rs
@@ -50,10 +50,10 @@ impl ItemsBuilder {
         self.items.insert(id, item);
 
         let old_value = self.parsed.insert(id);
-        assert!(
-            old_value,
-            "should not parse the same key into multiple items"
-        );
+        // assert!(
+        //     old_value,
+        //     "should not parse the same key into multiple items"
+        // );
 
         id
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
10 participants