[DebugInfo]Fix for redundant lexical block in debug info #895
Conversation
|
Gentle Ping @Jiel-nv! |
There was a problem hiding this comment.
This change works, the scope changes of the metadata look like shortcuts are made which bypassed the DILexicalBlock and subsequent DILocation mdnodes. However, isn't it better to avoid emitting those mdnodes because they are no longer referenced anywhere, in other words, those are unnecessary.
Actually, instead of checking STYPE on each cur_line_mdnode, it could be easier to set the root block mdnode to be the subprogram itself. Fortran doesn't have lexical block and nested lexical block. So it seems to me, a simple one line change might do the trick.
How about just use the the "parent_blk_mdnode" to replace the call to "lldbg_create_block_mdnode()".
I think this simple change works the same, i.e. redundant DWARF layer of lexical block will be removed. But a better looking debug metadata will be produced.
|
Thank you for your reviewing this!
With Fortran 2008 we can have lexical blocks with BLOCK statement: Anyways I've flipped the approach similar to what you mentioned and fortunately this covers this above test scenarios also. |
SouraVX
changed the title
|
Rebased on top of master, added test case. |
tools/flang2/flang2exe/lldebug.cpp
Outdated
| @@ -1810,7 +1810,9 @@ lldbg_emit_lexical_block(LL_DebugInfo *db, int sptr, int lineno, int findex, | |||
| NEEDB((db->blk_idx + 1), db->blk_tab, BLKINFO, db->blk_tab_size, | |||
| (db->blk_tab_size + 64)); | |||
| db->blk_tab[db->blk_idx].mdnode = | |||
| lldbg_create_block_mdnode(db, parent_blk_mdnode, lineno, 1, findex, ID++); | |||
| STYPEG(sptr) == ST_BLOCK ? lldbg_create_block_mdnode(db, | |||
| parent_blk_mdnode, lineno, 1, findex, ID++) | |||
There was a problem hiding this comment.
There are tabs here I think. Could you check and remove?
There was a problem hiding this comment.
Not visible from diff, but after setting list there is some special character "^I". Thanks for mentioning this!
Anyway should I squash(all the commits) it or do an extra commit for this formatting correction ?
There was a problem hiding this comment.
Please go ahead and squash.
Flang was creating a redundant lexical block at the beginning of of every subroutine. This was causing debuggers to incorrectly assume the scope of a variable to be this lexical block. This patch fixes this incorrect behavior by creating a lexical block iff there is actually present in source code.
|
Although this PR is already approved by @jiel-nv . @gklimowicz could you please run internal tests ? so that we can move ahead. |
|
Testing today... |
Flang was creating a redundant lexical block at the beginning of of every subroutine debug info. This was causing debuggers to incorrectly assume the scope of a variable to be this lexical block.
This patch fixes this incorrect behavior by replacing all the debug info references of this redundant lexical block to the actual subroutine.
Brief explanation and correct behavior can be found here:
http://lists.llvm.org/pipermail/flang-dev/2020-April/000277.html
https://patchwork.ozlabs.org/patch/436971/
After this, GDB is correctly able to show formal arguments for the case of pass by value parameters.