[PATCH] D36018: Make .file directive to have basename only

[Peter Collingbourne via llvm-commits]

ThinLTO uses the source filename to compute GUIDs for globals, so this
change will increase the likelihood of GUID collisions.

My understanding is that there are still known correctness bugs associated
with GUID collisions. Maybe Mehdi or Teresa know what the current status is.

Peter

On Wed, Aug 9, 2017 at 5:44 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> I think this is fine as I now see that there are other uses of
> source_filename and it is not clear that they all want just the file
> name itself.
>
> Peter, what do you think?
>
> Cheers,
> Rafael
>
> Taewook Oh via Phabricator via llvm-commits
> <llvm-commits at lists.llvm.org> writes:
>
> > twoh created this revision.
> >
> > Currently LLVM puts directory along with the filename in .file
> directive, but this behavior doesn't match gcc. There's a no clear
> description about which one is right (https://sourceware.org/
> binutils/docs/as/File.html#File), but one document (
> https://sourceware.org/gdb/current/onlinedocs/stabs/ELF-
> Linker-Relocation.html) suggests that STT_FILE symbol in elf file is
> expected to have basename only, which should have a same sting file .file
> directive according to (https://docs.oracle.com/cd/
> E26502_01/html/E28388/eoiyg.html).
> >
> > This also affects badly on the build system that uses hashing, as the
> directory info could be differnt from developer to developer even when
> they're working on same file.
> >
> >
> > https://reviews.llvm.org/D36018
> >
> > Files:
> >   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> >   test/CodeGen/X86/file-directive.ll
> >
> >
> > Index: test/CodeGen/X86/file-directive.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/X86/file-directive.ll
> > @@ -0,0 +1,14 @@
> > +; RUN: llc -mtriple=x86_64-linux-gnu -filetype=asm < %s | FileCheck %s
> --check-prefix=DIRECTIVE
> > +; RUN: llc -mtriple=x86_64-linux-gnu -filetype=obj < %s | llvm-readobj
> -symbols | FileCheck %s --check-prefix=STT-FILE
> > +
> > +; DIRECTIVE: .file "foobar"
> > +; STT-FILE: Name: foobar
> > +; STT-FILE-NEXT: Value: 0x0
> > +; STT-FILE-NEXT: Size: 0
> > +; STT-FILE-NEXT: Binding: Local
> > +; STT-FILE-NEXT: Type: File
> > +; STT-FILE-NEXT: Other: 0
> > +; STT-FILE-NEXT: Section: Absolute
> > +; STT-FILE-NOT: Name:
> > +
> > +source_filename = "/path/to/foobar"
> > Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > ===================================================================
> > --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > @@ -87,6 +87,7 @@
> >  #include "llvm/Support/ErrorHandling.h"
> >  #include "llvm/Support/Format.h"
> >  #include "llvm/Support/MathExtras.h"
> > +#include "llvm/Support/Path.h"
> >  #include "llvm/Support/TargetRegistry.h"
> >  #include "llvm/Support/Timer.h"
> >  #include "llvm/Support/raw_ostream.h"
> > @@ -272,7 +273,8 @@
> >    // don't, this at least helps the user find where a global came from.
> >    if (MAI->hasSingleParameterDotFile()) {
> >      // .file "foo.c"
> > -    OutStreamer->EmitFileDirective(M.getSourceFileName());
> > +    OutStreamer->EmitFileDirective(
> > +        llvm::sys::path::filename(M.getSourceFileName()));
> >    }
> >
> >    GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
> >
> >
> > Index: test/CodeGen/X86/file-directive.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/CodeGen/X86/file-directive.ll
> > @@ -0,0 +1,14 @@
> > +; RUN: llc -mtriple=x86_64-linux-gnu -filetype=asm < %s | FileCheck %s
> --check-prefix=DIRECTIVE
> > +; RUN: llc -mtriple=x86_64-linux-gnu -filetype=obj < %s | llvm-readobj
> -symbols | FileCheck %s --check-prefix=STT-FILE
> > +
> > +; DIRECTIVE: .file "foobar"
> > +; STT-FILE: Name: foobar
> > +; STT-FILE-NEXT: Value: 0x0
> > +; STT-FILE-NEXT: Size: 0
> > +; STT-FILE-NEXT: Binding: Local
> > +; STT-FILE-NEXT: Type: File
> > +; STT-FILE-NEXT: Other: 0
> > +; STT-FILE-NEXT: Section: Absolute
> > +; STT-FILE-NOT: Name:
> > +
> > +source_filename = "/path/to/foobar"
> > Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > ===================================================================
> > --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> > @@ -87,6 +87,7 @@
> >  #include "llvm/Support/ErrorHandling.h"
> >  #include "llvm/Support/Format.h"
> >  #include "llvm/Support/MathExtras.h"
> > +#include "llvm/Support/Path.h"
> >  #include "llvm/Support/TargetRegistry.h"
> >  #include "llvm/Support/Timer.h"
> >  #include "llvm/Support/raw_ostream.h"
> > @@ -272,7 +273,8 @@
> >    // don't, this at least helps the user find where a global came from.
> >    if (MAI->hasSingleParameterDotFile()) {
> >      // .file "foo.c"
> > -    OutStreamer->EmitFileDirective(M.getSourceFileName());
> > +    OutStreamer->EmitFileDirective(
> > +        llvm::sys::path::filename(M.getSourceFileName()));
> >    }
> >
> >    GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170809/047cb2de/attachment.html>