[llvm] r328208 - [DWARF] Fix mixing assembler -g with DWARF .file directives.
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 23 06:09:57 PDT 2018
> -----Original Message-----
> From: Benjamin Kramer [mailto:benny.kra at gmail.com]
> Sent: Friday, March 23, 2018 8:59 AM
> To: Robinson, Paul
> Cc: llvm-commits
> Subject: Re: [llvm] r328208 - [DWARF] Fix mixing assembler -g with DWARF
> .file directives.
>
> 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.
Okay. I believe gas won't complain as long as there isn't a ".file 1"
in the assembler source (it will silently accept ".file 2" and emit it
into the line table), but I'd say that behavior is a gas bug, and any
source exploiting that behavior is skating on thin ice.
In MC, I can envision allowing a label prior to the first .file, but
not actual instructions. Let me know what you think.
--paulr
>
> 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