[PATCH] D28095: [ELF] - Use information available from DW_AT_comp_dir attribute when doing error reporting.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:48-50
+    if (static_cast<const ELFSectionRef &>(Sec).getFlags() & ELF::SHF_ALLOC)
+      return static_cast<const ELFSectionRef &>(Sec).getOffset();
+    return 0;
----------------
davide wrote:
> you can probably simplify a bit using the ternary operator here, i.e.
> ```
> auto &patatino = static_cast<...>(Sec);
> return (patatino.getFlags() & SHF_ALLOC) ? patatino.getOffset(); 0;
> ```
Done.


================
Comment at: ELF/InputFiles.cpp:75
+
+  if (Dwarf.getNumCompileUnits())
+    CompilationDir = Dwarf.getCompileUnitAtIndex(0)->getCompilationDir();
----------------
evgeny777 wrote:
> Do we have a unit tests for zero number of CU ?
Example of test which does not have CUs but has debug line table is conflict.s:

> # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/conflict-debug.s -o %t-dbg.o
> # RUN: not ld.lld %t-dbg.o %t-dbg.o -o %t-dbg 2>&1 | FileCheck -check-prefix=DBGINFO %s

Where conflict-debug.s is:

```
.file 1 "conflict-debug.s"
.globl zed
.loc 1 4
zed:
  nop
```


================
Comment at: ELF/InputFiles.h:204-206
+
+  // Containing the working directory of the compilation command.
+  llvm::Optional<std::string> CompilationDir;
----------------
davide wrote:
> what do you mean with "compilation command" ?
umb at ubuntu:~/tests/181$ clang 111/1.cpp -g -o out

here I mean "clang 111/1.cpp -g -o out" is a compilation command.

I based on termonology from DW_AT_comp_dir description:
> 
> A DW_AT_comp_dir attribute whose value is a null-terminated string containing the current
> working directory of the **compilation command ** that produced this compilation unit

May be change it to next ?
> // Containing the compilation directory for CU.




================
Comment at: test/ELF/conflict2.s:9
+
+# Assembler file below was generated with following command line:
+# /home/bar/some_path clang foo/1.cpp -g -S -o asm.s, and then reduced manually.
----------------
davide wrote:
> s/Assembler file/The file/
> s/following/the following/
Done.


================
Comment at: test/ELF/conflict2.s:10
+# Assembler file below was generated with following command line:
+# /home/bar/some_path clang foo/1.cpp -g -S -o asm.s, and then reduced manually.
+# 1.cpp is a one line file: "int main() { return 0; }"
----------------
davide wrote:
> `/home/bar/some_path clang` ?
I meant that foo/1.cpp is placed in
/home/bar/some_path which is working dirrectory for a call, like:

```
bar at ubuntu:~/some_path$ clang foo/1.cpp -g -S -o asm.s
```

Updated the comment.


================
Comment at: test/ELF/conflict2.s:12
+# 1.cpp is a one line file: "int main() { return 0; }"
+# That way linker can extract current working directory of a compilation command from
+# DW_AT_comp_dir attribute for providing full patch when reporting errors.
----------------
davide wrote:
> s/linker/the linker/
Done.


================
Comment at: test/ELF/conflict2.s:13
+# That way linker can extract current working directory of a compilation command from
+# DW_AT_comp_dir attribute for providing full patch when reporting errors.
+
----------------
davide wrote:
> full patch or full path?
Fixed.


https://reviews.llvm.org/D28095