[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp created this revision.

The problem in PRR33930 <https://bugs.llvm.org/show_bug.cgi?id=33930> comes about because clang is cloning a function (to generate varargs thunks) before all the Metadata nodes are resolved. The value mapper, which is used by the cloner to deal with Medatdata nodes, asserts when it encounters temporary otherwise unresolved nodes.

Ideally we would improve varargs thunk generation to not use cloning, but in the meantime the fix I'm proposing here is to clone the function's DISubprogram node explicitly (the verifier insists on ever function having its own). The cloned node is then entered into the value map that is supplied to the cloner (which feeds it to the value mapper). This has the effect that the value mapper does not look any further when it's trying to map all the rest of the MD nodes and therefore does not run into any temporary MD nodes that still exist at than point (in the form of forward references to struct declarations).

Furthermore, there are unresolved DILocalVariable nodes referenced by dbg.value intrinsics, which causes the value mapper to assert when it is remapping the function's instructions' MDNodes. These nodes are normally resolved at the catch-all at the end of the module, but they can be safely resolved here, since they won't change any more. The value mapper will safely clone them (if necessary) and remap their operand nodes during the cloning process.

The only drawback is that the cloned function's (the thunk's) DILocalVariableNodes will not appear in the newly cloned DISubprogram's node variable list. Instead, the new node points to the original function's variable list. However, since the only difference between the original function and the thunk is the this-ptr adjustment, the resulting debug information is correct, at least in the test cases I have looked at.

Unfortunately this isn't the cleanest solution in the world. A different approach, deferring the creation of varargs thunks until the end of the module, is more invasive. The approach proposed here <https://reviews.llvm.org/D37038> duplicates Metadata. If anybody has any better ideas, please let me know.

Note that there are two patches here. One is an llvm patch that is NFC (making resolve() public) while the main one is a clang patch.


https://reviews.llvm.org/D39396

Files:
  include/llvm/IR/Metadata.h
  lib/CodeGen/CGVTables.cpp
  test/CodeGenCXX/tmp-md-nodes1.cpp
  test/CodeGenCXX/tmp-md-nodes2.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39396.120710.patch
Type: text/x-patch
Size: 4922 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171027/2d91a2b9/attachment.bin>