[PATCH] D44180: Improve --warn-symbol-ordering.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I'm not sure about the ICF behaviour change. If a user has two functions, and wants them to appear in two very different locations in the order file for some reason, ICF will silently break this order. Example order file:

  identical1
  not_identical
  identical2

With ICF enabled, identical1 and identical2 will be placed at the same location in the output (as is normal for ICF), and so there will not be one at the start and one at the end as expected. Therefore, we haven't created the order that was explicitly asked for. I think therefore it is reasonable to warn still in this case.

The flip-side is that if only one of identical1 or identical2 is specified in the order file, then we should probably not warn, since it is possible to order the symbols in this case! One possible solution is to record all symbols in the order file in sections which have been discarded due to ICF, then after completing the loop, looking for any such symbols whose section doesn't have the Priority that would be expected. It's not an ideal solution though.



================
Comment at: lld/ELF/Writer.cpp:1048
 
-      auto *D = dyn_cast<Defined>(Sym);
-      if (Config->WarnSymbolOrdering) {
-        if (Sym->isUndefined())
-          warn(File->getName() +
-               ": unable to order undefined symbol: " + Sym->getName());
-        else if (Sym->isShared())
-          warn(File->getName() +
-               ": unable to order shared symbol: " + Sym->getName());
-        else if (D && !D->Section)
-          warn(File->getName() +
-               ": unable to order absolute symbol: " + Sym->getName());
-        else if (D && !D->Section->Live)
-          warn(File->getName() +
-               ": unable to order discarded symbol: " + Sym->getName());
+      auto Warn = [&](StringRef Msg) {
+        if (Config->WarnSymbolOrdering)
----------------
We should probably avoid using "Warn" for this name, as it is too easy for a developer to accidentally call the wrong function (e.g. via type), and bypass the check. Perhaps "WarnOrder" or similar.


https://reviews.llvm.org/D44180