[llvm] r328208 - [DWARF] Fix mixing assembler -g with DWARF .file directives.
Benjamin Kramer via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 23 05:58:44 PDT 2018
It was in the wild, but I unilaterally decided that passing -g to
clang in that situation is a bad idea and fixed it. I'm still
undecided how far away from gcc+gas compatibility we want to go here.
On Fri, Mar 23, 2018 at 1:55 PM, <paul.robinson at sony.com> wrote:
>
>
>> -----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