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

[Rui Ueyama via llvm-commits]

I'm sorry I accidentally submitted this change along with some other
change. I'll create another patch now to address the review comments.


On Wed, Mar 7, 2018 at 9:18 AM Rui Ueyama via Phabricator <
reviews at reviews.llvm.org> wrote:

> This revision was not accepted when it landed; it landed in state "Needs
> Review".
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rLLD326911: Improve --warn-symbol-ordering. (authored by
> ruiu, committed by ).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D44180?vs=137412&id=137413#toc
>
> Repository:
>   rLLD LLVM Linker
>
> https://reviews.llvm.org/D44180
>
> Files:
>   ELF/Writer.cpp
>   test/ELF/symbol-ordering-file-icf.s
>   test/ELF/symbol-ordering-file-warnings.s
>
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1045,28 +1045,35 @@
>        SymbolOrderEntry &Ent = It->second;
>        Ent.Present = true;
>
> -      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 Warning = [&](StringRef Msg) {
> +        if (Config->WarnSymbolOrdering)
> +          warn(File->getName() + ": " + Msg + ": " + Sym->getName());
> +      };
> +
> +      if (Sym->isUndefined()) {
> +        Warning("unable to order undefined symbol");
> +        continue;
>        }
> -      if (!D)
> +      if (Sym->isShared()) {
> +        Warning("unable to order shared symbol");
>          continue;
> +      }
>
> -      if (auto *Sec = dyn_cast_or_null<InputSectionBase>(D->Section)) {
> -        int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];
> -        Priority = std::min(Priority, Ent.Priority);
> +      auto *Sec =
> dyn_cast_or_null<InputSectionBase>(cast<Defined>(Sym)->Section);
> +      if (!Sec) {
> +        Warning("unable to order absolute symbol");
> +        continue;
> +      }
> +      if (!Sec->Live) {
> +        if (Sec->Repl == Sec)
> +          Warning("unable to order discarded symbol");
> +        else
> +          Warning("unable to order a symbol merged by ICF");
> +        continue;
>        }
> +
> +      int &Priority = SectionOrder[cast<InputSectionBase>(Sec->Repl)];
> +      Priority = std::min(Priority, Ent.Priority);
>      }
>    }
>
> Index: test/ELF/symbol-ordering-file-warnings.s
> ===================================================================
> --- test/ELF/symbol-ordering-file-warnings.s
> +++ test/ELF/symbol-ordering-file-warnings.s
> @@ -109,7 +109,7 @@
>  # WARN-NOT:    warning:
>  # MISSING:     warning: symbol ordering file: no such symbol: missing
>  # MISSING2:    warning: symbol ordering file: no such symbol: missing_sym
> -# ICF:         warning: {{.*}}1.o: unable to order discarded symbol: icf2
> +# ICF:         warning: {{.*}}1.o: unable to order a symbol merged by
> ICF: icf2
>  # COMDAT:      warning: {{.*}}1.o: unable to order discarded symbol:
> comdat
>  # COMDAT-NEXT: warning: {{.*}}2.o: unable to order discarded symbol:
> comdat
>  # MULTI:       warning: {{.*}}2.o: unable to order absolute symbol: multi
> Index: test/ELF/symbol-ordering-file-icf.s
> ===================================================================
> --- test/ELF/symbol-ordering-file-icf.s
> +++ test/ELF/symbol-ordering-file-icf.s
> @@ -6,11 +6,10 @@
>  # RUN: ld.lld --icf=all --symbol-ordering-file %t.order -shared %t.o -o
> %t.so
>  # RUN: llvm-nm %t.so | FileCheck %s
>
> -## Check that after ICF merges 'foo' and 'zed' we still
> -## place them before 'bar', in according to ordering file.
> -# CHECK-DAG: 0000000000001000 T foo
> -# CHECK-DAG: 0000000000001000 T zed
> -# CHECK-DAG: 0000000000001004 T bar
> +## Check that we do not sort ICF'ed symbols 'foo' and 'zed'.
> +# CHECK-DAG: 0000000000001000 T bar
> +# CHECK-DAG: 0000000000001004 T foo
> +# CHECK-DAG: 0000000000001004 T zed
>
>  .section .text.foo,"ax", at progbits
>  .align 4
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180307/db2cdf93/attachment-0001.html>