[llvm] r328208 - [DWARF] Fix mixing assembler -g with DWARF .file directives.

via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 23 05:55:00 PDT 2018



> -----Original Message-----
> From: Benjamin Kramer [mailto:benny.kra at gmail.com]
> Sent: Friday, March 23, 2018 5:44 AM
> To: Robinson, Paul
> Cc: llvm-commits
> Subject: Re: [llvm] r328208 - [DWARF] Fix mixing assembler -g with DWARF
> .file directives.
> 
> It's still pretty easy to trigger the error message:
> 
> $ cat foo.s
> foo:
> .file   "a"
> .file   1 "b"
> 
> $ clang -g foo.s
> foo.s:3:1: error: file number already allocated
> .file   1 "b"
> ^
> 
> Is that intentional?

In this case, the assembler has emitted debug info for the label foo:
and then encountered the .file directives, which does seem like an
error of some kind to me.  Is there code like this in the wild?  If
this sequence has to be acceptable, that will be kind of a pain.
--paulr

> 
> On Thu, Mar 22, 2018 at 4:48 PM, Paul Robinson via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: probinson
> > Date: Thu Mar 22 08:48:01 2018
> > New Revision: 328208
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=328208&view=rev
> > Log:
> > [DWARF] Fix mixing assembler -g with DWARF .file directives.
> >
> > We were effectively overriding an explicit '.file' directive with info
> > for the assembler source.  That shouldn't happen.
> >
> > Fixes PR36636, really, even for .s files emitted by Clang.
> >
> > Differential Revision: https://reviews.llvm.org/D44265
> >
> > Modified:
> >     llvm/trunk/lib/MC/MCParser/AsmParser.cpp
> >     llvm/trunk/test/MC/AsmParser/directive_file-2.s
> >
> > Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=328208&r1=328207&r2=3
> 28208&view=diff
> >
> ==========================================================================
> ====
> > --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
> > +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Thu Mar 22 08:48:01 2018
> > @@ -311,6 +311,11 @@ private:
> >    }
> >    static void DiagHandler(const SMDiagnostic &Diag, void *Context);
> >
> > +  /// Should we emit DWARF describing this assembler source?  (Returns
> false if
> > +  /// the source has .file directives, which means we don't want to
> generate
> > +  /// info describing the assembler source itself.)
> > +  bool enabledGenDwarfForAssembly();
> > +
> >    /// \brief Enter the specified file. This returns true on failure.
> >    bool enterIncludeFile(const std::string &Filename);
> >
> > @@ -824,6 +829,19 @@ const AsmToken &AsmParser::Lex() {
> >    return *tok;
> >  }
> >
> > +bool AsmParser::enabledGenDwarfForAssembly() {
> > +  // Check whether the user specified -g.
> > +  if (!getContext().getGenDwarfForAssembly())
> > +    return false;
> > +  // If we haven't encountered any .file directives (which would imply
> that
> > +  // the assembler source was produced with debug info already) then
> emit one
> > +  // describing the assembler source file itself.
> > +  if (getContext().getGenDwarfFileNumber() == 0)
> > +
> getContext().setGenDwarfFileNumber(getStreamer().EmitDwarfFileDirective(
> > +        0, StringRef(), getContext().getMainFileName()));
> > +  return true;
> > +}
> > +
> >  bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
> >    // Create the initial section, if requested.
> >    if (!NoInitialTextSection)
> > @@ -837,7 +855,9 @@ bool AsmParser::Run(bool NoInitialTextSe
> >    SmallVector<AsmRewrite, 4> AsmStrRewrites;
> >
> >    // If we are generating dwarf for assembly source files save the
> initial text
> > -  // section and generate a .file directive.
> > +  // section.  (Don't use enabledGenDwarfForAssembly() here, as we
> aren't
> > +  // emitting any actual debug info yet and haven't had a chance to
> parse any
> > +  // embedded .file directives.)
> >    if (getContext().getGenDwarfForAssembly()) {
> >      MCSection *Sec = getStreamer().getCurrentSectionOnly();
> >      if (!Sec->getBeginSymbol()) {
> > @@ -848,8 +868,6 @@ bool AsmParser::Run(bool NoInitialTextSe
> >      bool InsertResult = getContext().addGenDwarfSection(Sec);
> >      assert(InsertResult && ".text section should not have debug info
> yet");
> >      (void)InsertResult;
> > -
> getContext().setGenDwarfFileNumber(getStreamer().EmitDwarfFileDirective(
> > -        0, StringRef(), getContext().getMainFileName()));
> >    }
> >
> >    // While we have input, parse each statement.
> > @@ -1784,7 +1802,7 @@ bool AsmParser::parseStatement(ParseStat
> >
> >      // If we are generating dwarf for assembly source files then gather
> the
> >      // info to make a dwarf label entry for this label if needed.
> > -    if (getContext().getGenDwarfForAssembly())
> > +    if (enabledGenDwarfForAssembly())
> >        MCGenDwarfLabelEntry::Make(Sym, &getStreamer(),
> getSourceManager(),
> >                                   IDLoc);
> >
> > @@ -2153,7 +2171,7 @@ bool AsmParser::parseStatement(ParseStat
> >
> >    // If we are generating dwarf for the current section then generate a
> .loc
> >    // directive for the instruction.
> > -  if (!ParseHadError && getContext().getGenDwarfForAssembly() &&
> > +  if (!ParseHadError && enabledGenDwarfForAssembly() &&
> >        getContext().getGenDwarfSectionSyms().count(
> >            getStreamer().getCurrentSectionOnly())) {
> >      unsigned Line;
> > @@ -3318,6 +3336,10 @@ bool AsmParser::parseDirectiveFile(SMLoc
> >      }
> >    }
> >
> > +  // In case there is a -g option as well as debug info from directive
> .file,
> > +  // we turn off the -g option, directly use the existing debug info
> instead.
> > +  getContext().setGenDwarfForAssembly(false);
> > +
> >    if (FileNumber == -1)
> >      getStreamer().EmitFileDirective(Filename);
> >    else {
> > @@ -3334,17 +3356,11 @@ bool AsmParser::parseDirectiveFile(SMLoc
> >        memcpy(SourceBuf, SourceString.data(), SourceString.size());
> >        Source = StringRef(SourceBuf, SourceString.size());
> >      }
> > -    // If there is -g option as well as debug info from directive file,
> > -    // we turn off -g option, directly use the existing debug info
> instead.
> > -    if (getContext().getGenDwarfForAssembly())
> > -      getContext().setGenDwarfForAssembly(false);
> > -    else {
> > -      Expected<unsigned> FileNumOrErr =
> getStreamer().tryEmitDwarfFileDirective(
> > -          FileNumber, Directory, Filename, CKMem, Source);
> > -      if (!FileNumOrErr)
> > -        return Error(DirectiveLoc, toString(FileNumOrErr.takeError()));
> > -      FileNumber = FileNumOrErr.get();
> > -    }
> > +    Expected<unsigned> FileNumOrErr =
> getStreamer().tryEmitDwarfFileDirective(
> > +        FileNumber, Directory, Filename, CKMem, Source);
> > +    if (!FileNumOrErr)
> > +      return Error(DirectiveLoc, toString(FileNumOrErr.takeError()));
> > +    FileNumber = FileNumOrErr.get();
> >    }
> >
> >    return false;
> >
> > Modified: llvm/trunk/test/MC/AsmParser/directive_file-2.s
> > URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/MC/AsmParser/directive_file-
> 2.s?rev=328208&r1=328207&r2=328208&view=diff
> >
> ==========================================================================
> ====
> > --- llvm/trunk/test/MC/AsmParser/directive_file-2.s (original)
> > +++ llvm/trunk/test/MC/AsmParser/directive_file-2.s Thu Mar 22 08:48:01
> 2018
> > @@ -1,4 +1,4 @@
> > -// RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
> > +// RUN: llvm-mc -g -triple i386-unknown-unknown %s | FileCheck %s
> >  // Test for Bug 11740
> >  // This testcase has two directive files,
> >  // when compiled with -g, this testcase will not report error,
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list