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

[Benjamin Kramer via llvm-commits]

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?

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=328208&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