[PATCH] D69650: [ELF] Suggest extern "C" when the definition is mangled while an undefined reference is not

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: lld/ELF/Relocations.cpp:705-706
+  unsigned len;
+  return def.consume_front("_Z") && !def.consumeInteger(10, len) &&
+         len == ref.size() && def.take_front(len) == ref;
+}
----------------
MaskRay wrote:
> dblaikie wrote:
> > MaskRay wrote:
> > > dblaikie wrote:
> > > > I'm not super comfortable with this hand-written demangling (maintenance burden/code duplication, etc) - is there something in the itanium demangling library that already does this/can be reused (or refactored so it can be reused)?
> > > One way that uses the LLVMDemangle library is to call ItaniumPartialDemangler::partialDemangle, then check whether the root node is of type KNodeType, then check its string.
> > > 
> > > The concern is that demangling every symbol table name may be slow. Similarly, in --version-script and --dynamic-list handling, we demangle all symbol table names on demand (only when extern `"C++"` is seen) because doing it unconditionally can be slow.
> > > 
> > This is only in the error case, right? We're already going to print an error to the user - so it seems we're already doing this lazily enough to not be doing it wastefully/needlessly (especially in a "Good" codepath).
> I agree there is a leaky abstraction here - it require some knowledge about the mangling scheme. The closest I can come up with is:
> 
>   lang=cpp
>   static bool canSuggestExternCXX(StringRef ref, StringRef def) {
>     llvm::ItaniumPartialDemangler d;
>     if (d.partialDemangle(def.str().c_str()))
>       return false;
>     char *buf = d.getFunctionName(nullptr, nullptr);
>     if (!buf)
>       return false;
>     bool ret = ref == buf;
>     free(buf);
>     return ret;
>   }
> 
> 
> I am not sure it is necessarily better.
> 
> The comment `def is in the form of _Z <length> <ref> <ignored>` is here to clarify the approach we take.
Opinions on the demangling method to choose? 1. as-is 2. ItaniumPartialDemangler




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69650