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

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
eddyz87 updated this revision to Diff 468102.
eddyz87 added a comment.
eddyz87 published this revision for review.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Rebase


Callsite `DISubprogram` entries are not generated for:

- builtin functions;
- external functions with reserved names (e.g. names starting from "__").

This limitation was added by the commit [1] as a workaround for the
situation described in [2] that triggered the IR verifier error.
The goal of the present commit is to lift this limitation by adjusting
the IR verifier logic.

The logic behind [1] is to avoid the following situation:

- a `DISubprogram` is added for some builtin function;
- there is some location where this builtin is also emitted by a transformation (w/o debug location);
- the `Verifier::visitCallBase` sees a call to a function with `DISubprogram` but w/o debug location and emits an error.

Here is an updated example of such situation taken from [2]:

  extern "C" int memcmp(void *, void *, long);
  
  struct a { int b; int c; int d; };
  
  struct e { int f[1000]; };
  
  bool foo(e g, e &h) {
    // DISubprogram for memcmp is created here when [1] is commented out
    return memcmp(&g, &h, sizeof(e));
  }
  
  bool bar(a &g, a &h) {
    // memcmp might be generated here by MergeICmps
    return g.b == h.b && g.c == h.c && g.d == h.d;
  }

This triggers the verifier error when:

- compiled for AArch64: `clang++ -c -g -Oz -target aarch64-unknown-linux-android21 test.cpp`;
- [1] check is commented out.

Instead of forbidding generation of `DISubprogram` entries as in [1]
one can instead adjust the verifier to additionally check if callee
has a body. Functions w/o bodies cannot be inlined and thus verifier
warning is not necessary.

E.g. `llvm::InlineFunction` requires functions for which
`GlobalValue::isDeclaration() == false`.

[1] 568db780bb7267651a902da8e85bc59fc89aea70 <https://reviews.llvm.org/rG568db780bb7267651a902da8e85bc59fc89aea70>
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=1022296


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136041

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  llvm/lib/IR/Verifier.cpp
  llvm/test/Verifier/callsite-dbgloc.ll


Index: llvm/test/Verifier/callsite-dbgloc.ll
===================================================================
--- llvm/test/Verifier/callsite-dbgloc.ll
+++ llvm/test/Verifier/callsite-dbgloc.ll
@@ -45,6 +45,9 @@
   call void @j()
 ; CHECK: inlinable function call in a function with debug info must have a !dbg location
   call void @k()
+; CHECK-NOT: inlinable function call in a function with debug info must have a !dbg location
+; CHECK-NOT: @i
+  call void (...) @i()
   ret void, !dbg !13
 }
 
Index: llvm/lib/IR/Verifier.cpp
===================================================================
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -3466,9 +3466,11 @@
   // Verify that each inlinable callsite of a debug-info-bearing function in a
   // debug-info-bearing function has a debug location attached to it. Failure to
   // do so causes assertion failures when the inliner sets up inline scope info
-  // (Interposable functions are not inlinable).
+  // (Interposable functions are not inlinable as are functions w/o
+  //  declarations).
   if (Call.getFunction()->getSubprogram() && Call.getCalledFunction() &&
       !Call.getCalledFunction()->isInterposable() &&
+      !Call.getCalledFunction()->isDeclaration() &&
       Call.getCalledFunction()->getSubprogram())
     CheckDI(Call.getDebugLoc(),
             "inlinable function call in a function with "
Index: clang/test/CodeGen/debug-info-extern-call.c
===================================================================
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -29,8 +29,8 @@
 // DECLS-FOR-EXTERN: [[FN1_TYPES]] = !{[[X_TYPE:![0-9]+]],
 // DECLS-FOR-EXTERN: [[X_TYPE]] = !DIDerivedType(tag: DW_TAG_typedef, name: "x",
 // DECLS-FOR-EXTERN-SAME: baseType: [[INT_TYPE]])
-// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
-// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
+// DECLS-FOR-EXTERN: !DISubprogram(name: "memcmp"
+// DECLS-FOR-EXTERN: !DISubprogram(name: "__some_reserved_name"
 
 // NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "fn1"
 // NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4228,17 +4228,11 @@
   if (Func->getSubprogram())
     return;
 
-  // Do not emit a declaration subprogram for a builtin, a function with nodebug
-  // attribute, or if call site info isn't required. Also, elide declarations
-  // for functions with reserved names, as call site-related features aren't
-  // interesting in this case (& also, the compiler may emit calls to these
-  // functions without debug locations, which makes the verifier complain).
-  if (CalleeDecl->getBuiltinID() != 0 || CalleeDecl->hasAttr<NoDebugAttr>() ||
+  // Do not emit a declaration subprogram for a function with nodebug
+  // attribute, or if call site info isn't required.
+  if (CalleeDecl->hasAttr<NoDebugAttr>() ||
       getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
     return;
-  if (CalleeDecl->isReserved(CGM.getLangOpts()) !=
-      ReservedIdentifierStatus::NotReserved)
-    return;
 
   // If there is no DISubprogram attached to the function being called,
   // create the one describing the function in order to have complete


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136041.468102.patch
Type: text/x-patch
Size: 3399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221016/def415f1/attachment-0001.bin>