[PATCH] D45711: [WebAssembly] Add an assertion for an invalid CFG

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 16 19:30:24 PDT 2018


dblaikie added a comment.

In https://reviews.llvm.org/D45711#1069489, @aheejin wrote:

> No, it was producing the correct answer. As long as `Header` was not empty, whether that part of the code was `Header` or `MBB` didn't affect the result of the code.


Why wouldn't that effect the result of the code? Header and MBB would have different instructions that could have different debug locations, right? So the code would produce different debug locations for the resulting instruction depending on whether it used Header on MBB?

Oh, I see what you mean - findDebugLoc doesn't actually use any of the members of the MachineBasicBlock it's called on (well, it uses the instr_end - so this code was invoking undefined behavior, presumably, when it passed an instruction iterator to an MBB that the instruction wasn't part of?)

So it got the right answer, despite the undefined behavior, except in the case where InsertPos was == end. So it sort of answers my original question (I phrased it poorly, because I assumed the implementation of findDebugLoc depended on the instance it was called on - which it should/dose, but not in a particularly meaningful way - only in a "invokes UB but probably is fine" kind of way)

Maybe this could be tested for, though - without debug info this findDebugLoc would potentially search forever or crash, perhaps? Since it would never compare == end (because it's a comparison across two different lists)? Seems like it'd be good to have that test case?


Repository:
  rL LLVM

https://reviews.llvm.org/D45711





More information about the llvm-commits mailing list