Fix incorrect DWARF 5 file name 0 when using -no-integrated-as

[osandov]

Summary

DWARF 5 specifies that in a line number program header in .debug_line,
directory entry 0 must be the compilation directory and file name entry
0 must be the compilation file name. Consider the following correct
output:

$ pwd
/home/osandov/test
$ cat foo/bar/baz.c
int my_var = 1;
$ clang -g -gdwarf-5 -c foo/bar/baz.c
$ readelf --debug-dump=line baz.o
...
 The Directory Table (offset 0x22, lines 1, columns 1):
  Entry Name
  0     (indirect line string, offset: 0x0): /home/osandov/test

 The File Name Table (offset 0x2e, lines 1, columns 3):
  Entry Dir     MD5                             Name
  0     0 0x436e7c8f87b60ba4ecab92f1eddfcbb4    (indirect line string, offset: 0x13): foo/bar/baz.c

However, when using -no-integrated-as, the compilation directory is
incorrectly given as the parent directory of the source file, and the
compilation file name is given relative to that:

$ clang -g -gdwarf-5 -no-integrated-as -c foo/bar/baz.c
$ readelf --debug-dump=line baz.o
...
 The Directory Table (offset 0x22, lines 1, columns 1):
  Entry Name
  0     (indirect line string, offset: 0x0): /home/osandov/test/foo/bar

 The File Name Table (offset 0x2e, lines 1, columns 3):
  Entry Dir     MD5                             Name
  0     0 0xb4cbdfedf192abeca40bb6878f7c6e43    (indirect line string, offset: 0x1b): baz.c

This is because the generated assembly .file directive does not split
the directory from the file name if -no-integrated-as is used:

$ clang -g -gdwarf-5 -S foo/bar/baz.c -o - | grep '\.file\s\+0'
        .file   0 "/home/osandov/test" "foo/bar/baz.c" md5 0xb4cbdfedf192abeca40bb6878f7c6e43
$ clang -g -gdwarf-5 -no-integrated-as -S foo/bar/baz.c -o - | grep '\.file\s\+0'
        .file   0 "/home/osandov/test/foo/bar/baz.c" md5 0xb4cbdfedf192abeca40bb6878f7c6e43

This is ostensibly for compatibility, since the [extended .file
syntax](https://sourceware.org/binutils/docs/as/File.html) that allows
specifying the directory separately was only recently added to the GNU
Assembler in binutils 2.35.

But, the md5 argument also relies on the extended syntax, so this
isn't actually improving compatibility when using DWARF 5. Fix this bug
by always using the directory argument for DWARF 5.

I noticed this bug when compiling the Linux kernel with Clang and DWARF
5, as the Linux kernel is compiled with -no-integrated-as.

Test Plan:
Updated CodeGen tests. Ran the reproducer above and saw that it matched
the -integrated-as version.

[dblaikie]

Please include a test case (hopefully/possibly the original patch that added the functionality for compatibility already included testing and this patch will need to update that testing)

Got a link to conversations, patches, etc, that confirm this claim:

This is ostensibly for compatibility, since the extended .file syntax that allows specifying the directory separately was only recently added to the GNU Assembler in binutils 2.35.

[osandov]

Got a link to conversations, patches, etc, that confirm this claim:

This behavior goes back to https://github.com/llvm/llvm-project/commit/1d617acef9f102629b736db06a5b5def0e116297, which states:

With r142300 but not this patch, clang -S may emit .s files that assemblers other than llvm-mc can't parse.

(llvm-mc got support in the previous commit, https://github.com/llvm/llvm-project/commit/40f8f2ff2452c32529a10ba504ba7fbecf8731cd.)

I'll work on a test case, thanks.

[osandov]

The existing CodeGen tests already cover this, so I updated them.

[dblaikie]

But, the md5 argument also relies on the extended syntax, so this isn't actually improving compatibility when using DWARF 5. Fix this bug by always using the directory argument for DWARF 5.

Hmm, I don't think this argument holds - we don't emit checksums by default, do we? They're opt-in, I think? (at the clang level)

So this change as proposed would break that backwards compatibility for assemblers for users who aren't opting in to checksums/haven't already broken it.

[osandov]

I'm not passing any flags to opt in to hashes, and I'm getting them:

$ clang --version
clang version 12.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ clang -g -gdwarf-5 -no-integrated-as -S foo/bar/baz.c -o - | grep '\.file'
        .file   "baz.c"
        .file   0 "/home/osandov/test/foo/bar/baz.c" md5 0xb4cbdfedf192abeca40bb6878f7c6e43

With the same results on the main branch. I don't have access to a non-Linux system to check there, though.

[osandov]

It is opt-in in the sense that you have to opt in to DWARF 5, though. And if you're asking for DWARF 5 with -no-integrated-as, then your assembler better support the directives needed for DWARF 5.

[dblaikie]

In D105662#2867839, @osandov wrote:

I'm not passing any flags to opt in to hashes, and I'm getting them:

$ clang --version
clang version 12.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ clang -g -gdwarf-5 -no-integrated-as -S foo/bar/baz.c -o - | grep '\.file'
        .file   "baz.c"
        .file   0 "/home/osandov/test/foo/bar/baz.c" md5 0xb4cbdfedf192abeca40bb6878f7c6e43

With the same results on the main branch. I don't have access to a non-Linux system to check there, though.

ah, right - fair enough then.

[osandov]

Thank you! How do I get this landed?

[MaskRay]

In D105662#2871563, @osandov wrote:

Thank you! How do I get this landed?

https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator

Usually the reviewer can help commit the change on behalf of the author.

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp:ShouldDisableDwarfDirectory sets -fno-dwarf-directory-asm for -fno-integrated-as.

With -fintegrated-as, UseDwarfDirectory is true (the default -fdwarf-directory-asm).

With -fno-integrated-as, UseDwarfDirectory is false (as if -fno-dwarf-directory-asm is specified).

-fdwarf-directory-asm appears to be a Clang specific option (not in GCC) for compatibility when GNU as did not support the new .file. I think with modern -gdwarf-5 -fno-dwarf-directory-asm should probably just be ignored.

[MaskRay]

llvm/test/CodeGen/Generic/dwarf-source.ll

14 ↗

(On Diff #357387)

I sent D105835 (driver option) to fix the original issue. Clang sets `UseDwarfDirectory propably.

A similar change is needed on llc side. llc should set UseDwarfDirectory to false only if -no-integrated-as and dwarf-version<5.

[MaskRay]

llvm/lib/MC/MCAsmStreamer.cpp

67–68

You may repurpose the patch to drop UseDwarfDirectory. The refactoring is still good to have.

llvm/lib/MC/MCAsmStreamer.cpp

class MCAsmStreamer final : public MCStreamer {
   unsigned ShowInst : 1;
   unsigned UseDwarfDirectory : 1;
 
   void EmitRegisterName(int64_t Register);
   void PrintQuotedString(StringRef Data, raw_ostream &OS) const;
   void printDwarfFileDirective(unsigned FileNo, StringRef Directory,
                                StringRef Filename,
                                Optional<MD5::MD5Result> Checksum,
                                Optional<StringRef> Source,
-                               bool UseDwarfDirectory,
                                raw_svector_ostream &OS) const;
   void emitCFIStartProcImpl(MCDwarfFrameInfo &Frame) override;
   void emitCFIEndProcImpl(MCDwarfFrameInfo &Frame) override;
 
 public:
   MCAsmStreamer(MCContext &Context, std::unique_ptr<formatted_raw_ostream> os,
                 bool isVerboseAsm, bool useDwarfDirectory,
                 MCInstPrinter *printer, std::unique_ptr<MCCodeEmitter> emitter,
                 std::unique_ptr<MCAsmBackend> asmbackend, bool showInst)

 
 void MCAsmStreamer::emitFileDirective(StringRef Filename) {
   assert(MAI->hasSingleParameterDotFile());
   OS << "\t.file\t";
   PrintQuotedString(Filename, OS);
   EmitEOL();
 }
 
-void MCAsmStreamer::printDwarfFileDirective(
-    unsigned FileNo, StringRef Directory, StringRef Filename,
-    Optional<MD5::MD5Result> Checksum, Optional<StringRef> Source,
-    bool UseDwarfDirectory, raw_svector_ostream &OS) const {
+void MCAsmStreamer::printDwarfFileDirective(unsigned FileNo,
+                                            StringRef Directory,
+                                            StringRef Filename,
+                                            Optional<MD5::MD5Result> Checksum,
+                                            Optional<StringRef> Source,
+                                            raw_svector_ostream &OS) const {
   SmallString<128> FullPathName;
 
-  if (!UseDwarfDirectory && !Directory.empty()) {
+  if (getContext().getDwarfVersion() < 5 && !UseDwarfDirectory &&
+      !Directory.empty()) {
     if (sys::path::is_absolute(Filename))
       Directory = "";
     else {
       FullPathName = Directory;
       sys::path::append(FullPathName, Filename);
       Directory = "";
       Filename = FullPathName;
     }
Expected<unsigned> MCAsmStreamer::tryEmitDwarfFileDirective(
   // Return early if this file is already emitted before or if target doesn't
   // support .file directive.
   if (NumFiles == Table.getMCDwarfFiles().size() ||
       !MAI->usesDwarfFileAndLocDirectives())
     return FileNo;
 
   SmallString<128> Str;
   raw_svector_ostream OS1(Str);
-  printDwarfFileDirective(FileNo, Directory, Filename, Checksum, Source,
-                          UseDwarfDirectory, OS1);
+  printDwarfFileDirective(FileNo, Directory, Filename, Checksum, Source, OS1);
 
   if (MCTargetStreamer *TS = getTargetStreamer())
     TS->emitDwarfFileDirective(OS1.str());
   else
     emitRawText(OS1.str());
 
   return FileNo;
 }
getContext().setMCLineTableRootFile(CUID, Directory, Filename, Checksum,
                                       Source);
 
   // Target doesn't support .loc/.file directives, return early.
   if (!MAI->usesDwarfFileAndLocDirectives())
     return;
 
   SmallString<128> Str;
   raw_svector_ostream OS1(Str);
-  printDwarfFileDirective(0, Directory, Filename, Checksum, Source,
-                          UseDwarfDirectory, OS1);
+  printDwarfFileDirective(0, Directory, Filename, Checksum, Source, OS1);
 
   if (MCTargetStreamer *TS = getTargetStreamer())
     TS->emitDwarfFileDirective(OS1.str());
   else
     emitRawText(OS1.str());
 }
 
 void MCAsmStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,