[PATCH] D90345: [DebugInfo] Fix ICE in DwarfCompileUnit::constructSubprogramScopeDIE

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 29 12:25:03 PDT 2020


dblaikie added a comment.

In D90345#2362082 <https://reviews.llvm.org/D90345#2362082>, @scott.linder wrote:

> In D90345#2362055 <https://reviews.llvm.org/D90345#2362055>, @scott.linder wrote:
>
>> In D90345#2360699 <https://reviews.llvm.org/D90345#2360699>, @dblaikie wrote:
>>
>>> IF we've so far successfully made the assumption that all DISubprograms have types - I'd say updating the verifier would be the way to go here. But I defer to @aprantl on verifier things, if he's got an overriding/differing opinion.
>>
>> Thank you for the feedback, I wasn't sure which way to lean so I just flipped a coin, as it were :^)
>>
>> I think ideally the types of these accessors would mirror the requirements and e.g. return an `Option` for anything not strictly required by the Verifier. I was also wondering if people would be amenable to changing the `LangRef` entries for the `DI*` metadata nodes to be explicit, for each field:
>>
>> - When it is required
>> - When it is forbidden (including e.g. mutual exclusion with other fields)
>> - Which "types" of metadata it can point to
>>
>> Currently it seems like we do this for some fields but not for others, and without anything in the type system to enforce things there isn't really any ground truth to work off of.
>>
>> If nobody has major objections I'd like to try to work on a patch when I have time.

Sounds plausible to me.

> Also, clangd seems to think this is the only use of `DISubprogram::getType()`, which doesn't seem quite right, but I went along with the idea that nowhere else is assuming `type:` is present.

Seems plausible to me - most of the DI info is only used once to generate the resulting DWARF or CodeView debug info at the end (yeah, I guess it's somewhat noteworthy that it's not used in the CodeView generation code).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90345/new/

https://reviews.llvm.org/D90345



More information about the llvm-commits mailing list