[PATCH] D27676: [ELF] - Use full object name if source file name exist when reporting errors.

[Rafael Avila de Espindola via llvm-commits]

I am OK with verbose errors, so LGTM, but please wait a day or so to see if
anyone thinks this is too verbose and has a suggestion for an heuristic.

Cheers,
Rafael


George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar, evgeny777.
>
> Previously if we were able to take source name, we used it exclusively for reporting.
> PR31354 case shows how confusing it can be:
>
> Error is:
> /usr/bin/ld: error: byte_copy.c:(.text+0x0): duplicate symbol 'byte_copy'
> /usr/bin/ld: error: byte_copy.c:(function byte_copy): previous definition was here
>
> After this patch output includes archive name if any. What makes clear what the error is about:
> lld.exe: error: usr/ports/sysutils/safecat/work/safecat-1.13/byte_copy.o(byte_copy.c):(.text+0x0): duplicate symbol 'byte_copy'
> lld.exe: error: usr/ports/sysutils/safecat/work/safecat-1.13/str.a(byte_copy.o)(byte_copy.c):(function byte_copy): previous definition was here
>
>
> https://reviews.llvm.org/D27676
>
> Files:
>   ELF/InputSection.cpp
>   test/ELF/Inputs/conflict-debug2.s
>   test/ELF/conflict.s
>   test/ELF/lto/combined-lto-object-name.ll
>   test/ELF/undef.s
>
>
> Index: test/ELF/undef.s
> ===================================================================
> --- test/ELF/undef.s
> +++ test/ELF/undef.s
> @@ -5,17 +5,17 @@
>  # RUN: llvm-ar rc %t2.a %t2.o
>  # RUN: not ld.lld %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
>  # RUN: not ld.lld -pie %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
> -# CHECK: error: undef.s:(.text+0x1): undefined symbol 'foo'
> -# CHECK: error: undef.s:(.text+0x6): undefined symbol 'bar'
> -# CHECK: error: undef.s:(.text+0x10): undefined symbol 'foo(int)'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x1): undefined symbol 'foo'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x6): undefined symbol 'bar'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x10): undefined symbol 'foo(int)'
>  # CHECK: error: {{.*}}2.a({{.*}}.o):(.text+0x0): undefined symbol 'zed2'
>  # CHECK: error: dir/undef-debug.s:3: undefined symbol 'zed3'
>  # CHECK: error: dir/undef-debug.s:7: undefined symbol 'zed4'
>  # CHECK: error: dir/undef-debug.s:11: undefined symbol 'zed5'
>  
>  # RUN: not ld.lld %t.o %t2.a -o %t.exe -no-demangle 2>&1 | \
>  # RUN:   FileCheck -check-prefix=NO-DEMANGLE %s
> -# NO-DEMANGLE: error: undef.s:(.text+0x10): undefined symbol '_Z3fooi'
> +# NO-DEMANGLE: error: {{.*}}.o(undef.s):(.text+0x10): undefined symbol '_Z3fooi'
>  
>  .file "undef.s"
>  
> Index: test/ELF/lto/combined-lto-object-name.ll
> ===================================================================
> --- test/ELF/lto/combined-lto-object-name.ll
> +++ test/ELF/lto/combined-lto-object-name.ll
> @@ -11,4 +11,4 @@
>    ret void
>  }
>  
> -; CHECK: error: ld-temp.o:(function _start): undefined symbol 'foo'
> +; CHECK: error: lto.tmp(ld-temp.o):(function _start): undefined symbol 'foo'
> Index: test/ELF/conflict.s
> ===================================================================
> --- test/ELF/conflict.s
> +++ test/ELF/conflict.s
> @@ -34,6 +34,14 @@
>  # DBGINFO:      conflict-debug.s:4: duplicate symbol 'zed'
>  # DBGINFO-NEXT: conflict-debug.s:4: previous definition was here
>  
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/conflict-debug2.s -o %t-dbg2.o
> +# RUN: echo "call zed" > %t-dbg1.s
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %t-dbg1.s -o %t-dbg1.o
> +# RUN: llvm-ar rcs %t-dbg2.a %t-dbg2.o
> +# RUN: not ld.lld %t-dbg2.a %t-dbg1.o %t-dbg2.o -o %t2 2>&1 | FileCheck -check-prefix=ARCHIVE2 %s
> +# ARCHIVE2:      {{.*}}-dbg2.o(conflict-debug.s):(.text+0x0): duplicate symbol 'zed'
> +# ARCHIVE2-NEXT: {{.*}}-dbg2.a({{.*}}-dbg2.o)(conflict-debug.s):(.text+0x0): previous definition was here
> +
>  .globl _Z3muldd, foo
>  _Z3muldd:
>  foo:
> Index: test/ELF/Inputs/conflict-debug2.s
> ===================================================================
> --- test/ELF/Inputs/conflict-debug2.s
> +++ test/ELF/Inputs/conflict-debug2.s
> @@ -0,0 +1,4 @@
> +.file "conflict-debug.s"
> +.globl zed
> +zed:
> +  nop
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -221,10 +221,10 @@
>      return LineInfo;
>  
>    // File->SourceFile contains STT_FILE symbol that contains a
> -  // source file name. If it's missing, we use an object file name.
> -  std::string SrcFile = File->SourceFile;
> -  if (SrcFile.empty())
> -    SrcFile = toString(File);
> +  // source file name. We add it if exist.
> +  std::string SrcFile = toString(File);
> +  if (!File->SourceFile.empty())
> +    SrcFile += ("(" + File->SourceFile + ")").str();
>  
>    // Find a function symbol that encloses a given location.
>    for (SymbolBody *B : File->getSymbols())
>
>
> Index: test/ELF/undef.s
> ===================================================================
> --- test/ELF/undef.s
> +++ test/ELF/undef.s
> @@ -5,17 +5,17 @@
>  # RUN: llvm-ar rc %t2.a %t2.o
>  # RUN: not ld.lld %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
>  # RUN: not ld.lld -pie %t.o %t2.a %t3.o -o %t.exe 2>&1 | FileCheck %s
> -# CHECK: error: undef.s:(.text+0x1): undefined symbol 'foo'
> -# CHECK: error: undef.s:(.text+0x6): undefined symbol 'bar'
> -# CHECK: error: undef.s:(.text+0x10): undefined symbol 'foo(int)'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x1): undefined symbol 'foo'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x6): undefined symbol 'bar'
> +# CHECK: error: {{.*}}.o(undef.s):(.text+0x10): undefined symbol 'foo(int)'
>  # CHECK: error: {{.*}}2.a({{.*}}.o):(.text+0x0): undefined symbol 'zed2'
>  # CHECK: error: dir/undef-debug.s:3: undefined symbol 'zed3'
>  # CHECK: error: dir/undef-debug.s:7: undefined symbol 'zed4'
>  # CHECK: error: dir/undef-debug.s:11: undefined symbol 'zed5'
>  
>  # RUN: not ld.lld %t.o %t2.a -o %t.exe -no-demangle 2>&1 | \
>  # RUN:   FileCheck -check-prefix=NO-DEMANGLE %s
> -# NO-DEMANGLE: error: undef.s:(.text+0x10): undefined symbol '_Z3fooi'
> +# NO-DEMANGLE: error: {{.*}}.o(undef.s):(.text+0x10): undefined symbol '_Z3fooi'
>  
>  .file "undef.s"
>  
> Index: test/ELF/lto/combined-lto-object-name.ll
> ===================================================================
> --- test/ELF/lto/combined-lto-object-name.ll
> +++ test/ELF/lto/combined-lto-object-name.ll
> @@ -11,4 +11,4 @@
>    ret void
>  }
>  
> -; CHECK: error: ld-temp.o:(function _start): undefined symbol 'foo'
> +; CHECK: error: lto.tmp(ld-temp.o):(function _start): undefined symbol 'foo'
> Index: test/ELF/conflict.s
> ===================================================================
> --- test/ELF/conflict.s
> +++ test/ELF/conflict.s
> @@ -34,6 +34,14 @@
>  # DBGINFO:      conflict-debug.s:4: duplicate symbol 'zed'
>  # DBGINFO-NEXT: conflict-debug.s:4: previous definition was here
>  
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/conflict-debug2.s -o %t-dbg2.o
> +# RUN: echo "call zed" > %t-dbg1.s
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %t-dbg1.s -o %t-dbg1.o
> +# RUN: llvm-ar rcs %t-dbg2.a %t-dbg2.o
> +# RUN: not ld.lld %t-dbg2.a %t-dbg1.o %t-dbg2.o -o %t2 2>&1 | FileCheck -check-prefix=ARCHIVE2 %s
> +# ARCHIVE2:      {{.*}}-dbg2.o(conflict-debug.s):(.text+0x0): duplicate symbol 'zed'
> +# ARCHIVE2-NEXT: {{.*}}-dbg2.a({{.*}}-dbg2.o)(conflict-debug.s):(.text+0x0): previous definition was here
> +
>  .globl _Z3muldd, foo
>  _Z3muldd:
>  foo:
> Index: test/ELF/Inputs/conflict-debug2.s
> ===================================================================
> --- test/ELF/Inputs/conflict-debug2.s
> +++ test/ELF/Inputs/conflict-debug2.s
> @@ -0,0 +1,4 @@
> +.file "conflict-debug.s"
> +.globl zed
> +zed:
> +  nop
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -221,10 +221,10 @@
>      return LineInfo;
>  
>    // File->SourceFile contains STT_FILE symbol that contains a
> -  // source file name. If it's missing, we use an object file name.
> -  std::string SrcFile = File->SourceFile;
> -  if (SrcFile.empty())
> -    SrcFile = toString(File);
> +  // source file name. We add it if exist.
> +  std::string SrcFile = toString(File);
> +  if (!File->SourceFile.empty())
> +    SrcFile += ("(" + File->SourceFile + ")").str();
>  
>    // Find a function symbol that encloses a given location.
>    for (SymbolBody *B : File->getSymbols())