[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D100567#2695827 <https://reviews.llvm.org/D100567#2695827>, @dblaikie wrote:

> Generally looks OK, but could you explain more about the problems with/motivation for the declaration/definition issue? (probably worth noting in the patch description maybe comment in code and test)

First about testing Fn->getSubprogram(), the reason is to avoid repeatedly attempt to generate debuginfo, e.g.,

  extern int foo(int);
  long test() { ... &foo ... &foo ... &foo ... }

Without checking it will try to generate the debuginfo three times although later it is deduplicated, but still a waste.

But Fn->getSubprogram() alone may cause issues if the debuginfo has been generated for a definition,

  extern int do_work2(int);
  long bpf_helper2(void *callback_fn);
  int do_work2(int arg) {
    return arg;
  }
  long prog2() {
    return bpf_helper2(&do_work2);
  }

We will have a segfault

  #3 0x0000000001bf2138 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0                                                                         

#4 0x00007f93e4fdfb20 __restore_rt sigaction.c:0:0                                                                                                         
 #5 0x0000000001506f70 llvm::Value::getMetadata(unsigned int) const (/home/yhs/work/llvm-project/llvm/build.cur/install/bin/clang-13+0x1506f70)             
 #6 0x00000000015074db llvm::Function::getSubprogram() const (/home/yhs/work/llvm-project/llvm/build.cur/install/bin/clang-13+0x15074db)                    
 #7 0x00000000021ff5cf clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(clang::DeclRefExpr const*) (/home/yhs/work/llvm-project/llvm/build.cur/install/bin
/clang-13+0x21ff5cf)                                                                                                                                        
 #8 0x00000000021fd866 clang::CodeGen::CodeGenFunction::EmitLValue(clang::Expr const*) (/home/yhs/work/llvm-project/llvm/build.cur/install/bin/clang-13+0x21
fd866)

So we need to stop calling Fn->getSubprogram() if FD->isDefined() is true. Hence I added FD->isDefined().

In summary, the new code

  if (!FD->isDefined() && !Fn->getSubprogram()) {
    CGM.getModuleDebugInfo()->EmitFunctionDecl(FD, FD->getLocation(), T,
                                               Fn);
  }

is an optimization to avoid repeatedly generating the debuginfo. If FD->isDefined(), stop,
otherwise, if Fn->getSubprogram() which means having generated, stop. Otherwise, generate one.

Note that my previous code without " if (!FD->isDefined() && !Fn->getSubprogram())" totally
working fine, I just feel a little bit waste.

I didn't debug why we have error for this test and you may have a better idea from the above
stack trace.

  extern int do_work2(int);
  long bpf_helper2(void *callback_fn);
  int do_work2(int arg) {
    return arg;
  }
  long prog2() {
    return bpf_helper2(&do_work2);
  }

Apparently, do_work2 already generated a subprogram debuginfo when inside prog2() another attempt tried to check whether the subprogram has been generated or not.
I have added a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567