[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

Tue Oct 18 08:42:55 PDT 2022

dblaikie accepted this revision.
dblaikie added a comment.

In D136041#3864171 <https://reviews.llvm.org/D136041#3864171>, @yonghong-song wrote:

> In D136041#3863748 <https://reviews.llvm.org/D136041#3863748>, @dblaikie wrote:
>
>> Hmm - this does mean linking IR can produce invalid code, though, right (you link in a definition of the function, so what was valid is now invalid - because it now has a definition, can be inlined, etc)? Is that new? concerning?
>
> @dblaikie do you have a concrete example to illustrate the problem?

One translation unit with a call to a function declaration, that call has no !dbg attachment.
Another translation unit with the definition of that declared function.

As-is, this would not cause any verifier error or backend debug info assertion.
Link the two together and now it's a verifier error - and, depending on the contents of the function definition, maybe a backend debug info assertion.

In D136041#3864782 <https://reviews.llvm.org/D136041#3864782>, @eddyz87 wrote:

> In D136041#3863748 <https://reviews.llvm.org/D136041#3863748>, @dblaikie wrote:
>
>> Hmm - this does mean linking IR can produce invalid code, though, right (you link in a definition of the function, so what was valid is now invalid - because it now has a definition, can be inlined, etc)? Is that new? concerning?
>
> As far as I understand this <https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160425/350532.html> discussion the check in question is "best effort".

Fair point

> My change further narrows conditions when verifier would report an error, thus it should not add any new failures to the existing code.

No, the opposite.

> But yes, hypothetically there might be a situation when old version of the check would have caught a miss-behaving transformation at an earlier stage (before linking IR rather then after).

Right - now things that were invalid categorically, are now valid, unless you link them. Which is a bit tricky.

Ah well, as you say - it's best effort.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136041