[DebugInfo] Enforce implicit constraints on `distinct` MDNodes
authorScott Linder <Scott.Linder@amd.com>
Thu, 4 Nov 2021 16:52:32 +0000 (16:52 +0000)
committerScott Linder <Scott.Linder@amd.com>
Tue, 9 Nov 2021 18:19:11 +0000 (18:19 +0000)
commitee7652569854af567ba83e5255d70e80cc8619a1
tree40c5c10d15d1e6b384e1853f7d8bc08913632ca8
parent181763d4756a2d1d6cb33b3d21fa4f0d3460e34b
[DebugInfo] Enforce implicit constraints on `distinct` MDNodes

Add UNIQUED and DISTINCT properties in Metadata.def and use them to
implement restrictions on the `distinct` property of MDNodes:

* DIExpression can currently be parsed from IR or read from bitcode
  as `distinct`, but this property is silently dropped when printing
  to IR. This causes accepted IR to fail to round-trip. As DIExpression
  appears inline at each use in the canonical form of IR, it cannot
  actually be `distinct` anyway, as there is no syntax to describe it.
* Similarly, DIArgList is conceptually always uniqued. It is currently
  restricted to only appearing in contexts where there is no syntax for
  `distinct`, but for consistency it is treated equivalently to
  DIExpression in this patch.
* DICompileUnit is already restricted to always being `distinct`, but
  along with adding general support for the inverse restriction I went
  ahead and described this in Metadata.def and updated the parser to be
  general. Future nodes which have this restriction can share this
  support.

The new UNIQUED property applies to DIExpression and DIArgList, and
forbids them to be `distinct`. It also implies they are canonically
printed inline at each use, rather than via MDNode ID.

The new DISTINCT property applies to DICompileUnit, and requires it to
be `distinct`.

A potential alternative change is to forbid the non-inline syntax for
DIExpression entirely, as is done with DIArgList implicitly by requiring
it appear in the context of a function. For example, we would forbid:

    !named = !{!0}
    !0 = !DIExpression()

Instead we would only accept the equivalent inlined version:

    !named = !{!DIExpression()}

This essentially removes the ability to create a `distinct` DIExpression
by construction, as there is no syntax for `distinct` inline. If this
patch is accepted as-is, the result would be that the non-canonical
version is accepted, but the following would be an error and produce a diagnostic:

    !named = !{!0}
    ; error: 'distinct' not allowed for !DIExpression()
    !0 = distinct !DIExpression()

Also update some documentation to consistently use the inline syntax for
DIExpression, and to describe the restrictions on `distinct` for nodes
where applicable.

Reviewed By: StephenTozer, t-tye

Differential Revision: https://reviews.llvm.org/D104827
18 files changed:
llvm/docs/LangRef.rst
llvm/docs/SourceLevelDebugging.rst
llvm/include/llvm/AsmParser/LLParser.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/include/llvm/IR/Metadata.def
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/MIRParser/MIParser.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/lib/IR/Metadata.cpp
llvm/test/Assembler/invalid-diarglist-outside-function.ll [new file with mode: 0644]
llvm/test/Assembler/invalid-diexpression-distinct.ll [new file with mode: 0644]
llvm/test/Bitcode/DIExpression-is-distinct-upgrade.ll [new file with mode: 0644]
llvm/test/Bitcode/DIExpression-is-distinct-upgrade.ll.bc [new file with mode: 0644]