Emit source map information for control flow structures

[kripken]

With this, the sourcemap testcase outputs the exact same thing as the input.

Followup to #5504

cc @JesseCodeBones

[JesseCodeBones]

Hi Alon,
I found if there is duplicate debug location for different instructions, the binaryen will dismiss some of them, here is the lit test, I am still finding the root cause and solution.
I donot know if it is meaningful in real world, and I think for some dynamic language concepts, like closure concept, it could happen, WDYT for such use case?

;; RUN: wasm-opt %s -o %t.wasm -osm %t.map -g -q
;; RUN: wasm-opt %t.wasm -ism %t.map -q -o - -S | filecheck %s
(module
  (func $foo (param $a i32) (param $b i32) (result i32)
    (return
        (i32.add 
            (local.get $a)
            (local.get $b)
        )
    )
  )
  (func $bar (param $a i32)
    ;;@ src.cpp:10:1
    (local.set $a
     (i32.const 1)
    )
    ;;@ src.cpp:10:1
    (drop
        ;;@ src.cpp:10:1
        (call $foo
            ;;@ src.cpp:20:1
            (local.get $a)
            ;;@ src.cpp:30:1
            (i32.const 42)
        )
    )
    (return)
  )
)

;; CHECK:  (module
;; CHECK-NEXT:   (type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
;; CHECK-NEXT:    (type $i32_=>_none (func (param i32)))
;; CHECK-NEXT:    (func $foo (param $a i32) (param $b i32) (result i32)
;; CHECK-NEXT:     (return
;; CHECK-NEXT:      (i32.add
;; CHECK-NEXT:       (local.get $a)
;; CHECK-NEXT:       (local.get $b)
;; CHECK-NEXT:      )
;; CHECK-NEXT:     )
;; CHECK-NEXT:    )
;; CHECK-NEXT:    (func $bar (param $a i32)
;; CHECK-NEXT:     ;;@ src.cpp:10:1
;; CHECK-NEXT:     (local.set $a
;; CHECK-NEXT:      (i32.const 1)
;; CHECK-NEXT:     )
;; CHECK-NEXT:     (drop
;; CHECK-NEXT:      (call $foo
;; CHECK-NEXT:       ;;@ src.cpp:20:1
;; CHECK-NEXT:       (local.get $a)
;; CHECK-NEXT:       ;;@ src.cpp:30:1
;; CHECK-NEXT:       (i32.const 42)
;; CHECK-NEXT:      )
;; CHECK-NEXT:     )
;; CHECK-NEXT:     (return)
;; CHECK-NEXT:    )
;; CHECK-NEXT:   )

Regards

[kripken]

@JesseCodeBones That does look like a real bug. Source maps support referring to the same source line from multiple places. And I agree it can happen in real-world code because sometimes a single source line ends up turned into multiple wasm lines, like you said. If you can find a fix that would be great!

I think that is separate from this PR, though? I see the same results on that testcase with this PR or not. Does this PR look ok to you to land?

[kripken]

cc @aheejin - not sure if you're familiar with this part of the debugging logic (it's source maps and not DWARF) but maybe you're interested in it too.

[JesseCodeBones]

@JesseCodeBones That does look like a real bug. Source maps support referring to the same source line from multiple places. And I agree it can happen in real-world code because sometimes a single source line ends up turned into multiple wasm lines, like you said. If you can find a fix that would be great!

I think that is separate from this PR, though? I see the same results on that testcase with this PR or not. Does this PR look ok to you to land?

yeah, I can handle it after this pr, it may take sometime and I have some personal issue to be handled, I suppose it is not an urgen or blocking issue

[kripken]

Sounds good, thanks, and yes, this is not urgent of course.

[aheejin]

I'm not very familiar with this part of the code. A few questions:

• Why didn't we print the debug location for control flow structure before?
• It looks this src.cpp:20:1 is the debug info for not if but i32.add, given that we didn't print debug info for control flow structures. Then why was it printed before if and not before i32.add?

[kripken]

Unfortunately I don't think anyone is very familiar with it 😄 It was written a long time ago (and not by me).

I can't see a reason for not printing it for control flow structures before. That might have just been an omission.

I'm not sure about the second question. I know there are some heuristics for how to print and emit stuff, that involve looking at recent instructions or such, so probably related to that?

Overall, my approach here considering this unfamiliar code is to get more tests on it, and improve on them. This PR seems to do that as after it we appear to perfectly roundtrip the input.