Skip to content

[bug] Split doesn't work with main function #476

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

Closed
willemneal opened this issue Feb 8, 2019 · 14 comments · Fixed by #489
Closed

[bug] Split doesn't work with main function #476

willemneal opened this issue Feb 8, 2019 · 14 comments · Fixed by #489
Labels

Comments

@willemneal
Copy link
Contributor

It seems that adding a main function makes String.split not work.

Here is the working example: https://webassembly.studio/?f=mn7zhulmmq9

If you build and runtestSplit it returns 13 to the console.

export function testSplit(): i32 {
  let str = "hell o o o o - - - - - - - -";
  let strs = str.split(" ");
  return strs.length;
}
// export function main(): void {

// }

If you uncomment the main function, build, and rerun it returns be 1.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 8, 2019

Hmm, it should be relate to some compile generated wrapper for start. Could you share generated wast file via pastebin or gist file? Because WebAssembly Studio currently broken if you clear cache as in my scenario (but I already send PR for fixing that)

@willemneal
Copy link
Contributor Author

Well Since I had it up from the other day it was working, but now it doesn't. Luckily it had the file cached: https://gist.github.com/willemneal/da038a47bb33c4cf25b822cb3d3bdbda#file-main-wat-L2555

@MaxGraey
Copy link
Member

MaxGraey commented Feb 8, 2019

Thanks, but I launched WAS locally already)

@MaxGraey
Copy link
Member

MaxGraey commented Feb 8, 2019

It seems problem here:

(func $main/testSplit (export "testSplit") (type $t0) (result i32)
    i32.const 1
    set_global $g0 ;; <---- hmm
    call $~lib/string/String#split|trampoline
    i32.load offset=4)

(global $g0 (mut i32) (i32.const 0))
(global $g1 (mut i32) (i32.const 0))
(elem (i32.const 0) $start)
(data (i32.const 8) "\1c\00\00\00h\00e\00l\00l\00 \00o\00 \00o\00 \00o\00 \00o\00 \00-\00 \00-\00 \00-\00 \00-\00 \00-\00 \00-\00 \00-\00 \00-")

cc @dcodeIO

@willemneal willemneal changed the title [bug] Split doesn't work with main class [bug] Split doesn't work with main function Feb 8, 2019
@MaxGraey
Copy link
Member

MaxGraey commented Feb 8, 2019

Well Since I had it up from the other day it was working, but now it doesn't

Some problems in jsdelivr CDN.

@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2019

Wasm Studio output is rather useless as it optimizes and loses names. When I paste

export function testSplit(): i32 {
  let str = "hell o o o o - - - - - - - -";
  let strs = str.split(" ");
  return strs.length;
}
export function main(): void {}

into tests/compiler/empty.ts and run npm run test:compiler empty instead, which I'd recommend, I get

 (func $empty/testSplit (; 18 ;) (type $i) (result i32)
  (local $0 i32)
  (local $1 i32)
  (local $2 i32)
  i32.const 8
  local.set $0
  block (result i32)
   i32.const 1
   global.set $~argc
   local.get $0
   i32.const 72
   i32.const 0
   call $~lib/string/String#split|trampoline
  end
  local.set $1
  block $~lib/array/Array<String>#get:length|inlined.0 (result i32)
   local.get $1
   local.set $2
   local.get $2
   i32.load offset=4
  end
 )
 (func $empty/main (; 19 ;) (type $_)
  global.get $~started
  i32.eqz
  if
   call $start
   i32.const 1
   global.set $~started
  end
 )

which is much easier to look at. Without main it reads:

 (func $empty/testSplit (; 18 ;) (type $i) (result i32)
  (local $0 i32)
  (local $1 i32)
  (local $2 i32)
  i32.const 8
  local.set $0
  block (result i32)
   i32.const 1
   global.set $~argc
   local.get $0
   i32.const 72
   i32.const 0
   call $~lib/string/String#split|trampoline
  end
  local.set $1
  block $~lib/array/Array<String>#get:length|inlined.0 (result i32)
   local.get $1
   local.set $2
   local.get $2
   i32.load offset=4
  end
 )

Anything specific to look for here? (didn't check)

@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2019

Also, with the snipped copied into empty, doing

npm run test:compiler empty -- --create

then uncommenting main and doing

npm run test:compiler empty

again shows no significant differences that could lead to the reported issue.

--- expected
+++ actual
@@ -28,14 +28,16 @@
  (global $~lib/internal/arraybuffer/MAX_BLENGTH i32 (i32.const 1073741816))
  (global $~lib/builtins/i32.MAX_VALUE i32 (i32.const 2147483647))
  (global $~lib/internal/string/HEADER_SIZE i32 (i32.const 4))
  (global $~lib/internal/string/MAX_LENGTH i32 (i32.const 536870910))
  (global $~argc (mut i32) (i32.const 0))
+ (global $~started (mut i32) (i32.const 0))
  (global $HEAP_BASE i32 (i32.const 284))
  (export "memory" (memory $0))
  (export "table" (table $0))
  (export "testSplit" (func $empty/testSplit))
+ (export "main" (func $empty/main))
  (func $~lib/internal/arraybuffer/computeSize (; 1 ;) (type $ii) (param $0 i32) (result i32)
   i32.const 1
   i32.const 32
   local.get $0
   global.get $~lib/internal/arraybuffer/HEADER_SIZE
@@ -2586,8 +2588,19 @@
    local.set $2
    local.get $2
    i32.load offset=4
   end
  )
- (func $null (; 19 ;) (type $_)
+ (func $empty/main (; 19 ;) (type $_)
+  global.get $~started
+  i32.eqz
+  if
+   call $start
+   i32.const 1
+   global.set $~started
+  end
  )
+ (func $start (; 20 ;) (type $_)
+ )
+ (func $null (; 21 ;) (type $_)
+ )
 )

(in your program, did you maybe forget to call main? without, some stuff might not be properly started yet, because start and thus all top-level statements do not execute)

@MaxGraey
Copy link
Member

MaxGraey commented Feb 9, 2019

@willemneal
As Daniel mentioned you should manually call exports.main before other exported methods and I confirm this works as expected.

@dcodeIO
I totally forget about that. This mentioned here btw. But it seems need better clarify

@dcodeIO
Copy link
Member

dcodeIO commented Feb 9, 2019

For context: The point of exporting a main function is that, if you don't import your memory, there is no access to the module's memory until the point when all top level statements have executed and the instantiation returns. That makes it impossible to log a string from a top-level statement for example, because there's nothing one could read its bytes from. By delaying top-level statements until main has been called, this problem is mitigated because the instantiation first returns.

Aside from better documentation, which is always good, maybe it'll help to name it start again .. or even introduce a @start decorator with no particular name being required to avoid accidental use of the feature altogether?

@MaxGraey
Copy link
Member

@willemneal So this not bug. Is calling main before testSplit resolve you problem?

@willemneal
Copy link
Contributor Author

Well the problem arose because I was trying to use main like a c program and wanted to pass an argv string to be split and it wouldn't work. So main should be empty? If so that should be documented and is not a bug.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 11, 2019

Yeah, need better document this because main have special meaning. Instead main, you could use init for your purposes.

@willemneal
Copy link
Contributor Author

Yeah, I just wanted to use main for my students who are used to C.

@dcodeIO
Copy link
Member

dcodeIO commented Feb 19, 2019

FYI: While I was working on the resolver branch I had to do something about the main function anyway and decided to change start function overrides to use an explicit @start decorator, independently of the function's name.

@dcodeIO dcodeIO mentioned this issue Feb 19, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants