Only warn about DWARF2 supporting one section per compilation unit for code sections

[dim]

Summary

r211273 introduced a new error "DWARF2 only supports one section per compilation unit" when multiple sections are encountered in asm, and when compiling for DWARF2.

However, apparently this limitation only applies for code sections, e.g. those with Type == ELF::SHT_PROGBITS. Therefore, only emit the warning for those types of sections.

[dblaikie]

Should we add that filter elsewhere rather than here? (eg: we shouldn't be adding such non-text sections into the Sections map if they're not part of teh DWARF anyway, right?)

& this could use a test case as well.

[dim]

In D6379#95991, @dblaikie wrote:

Should we add that filter elsewhere rather than here? (eg: we shouldn't be adding such non-text sections into the Sections map if they're not part of teh DWARF anyway, right?)

I realized this review was still in the system from a long time ago, and the situation still hasn't changed: this warning still pops up regularly. David, I'm trying to interpret your comment here, how would we prevent inserting such sections?

Note that in FreeBSD, we commonly see this warning due to .note.GNU-stack sections being artificially inserted at the end of .S files, for example here: https://github.com/freebsd/freebsd/blob/master/libexec/rtld-elf/amd64/rtld_start.S#L171 . Since the file starts with a .text section, which has progbits type, the last line with .section .note.GNU-stack,"",%progbits triggers this warning, which may even be right. Not sure how you should insert such a section otherwise.

& this could use a test case as well.

IIRC I had one at some point, maybe I can come up with a new one, otherwise I will just abandon this review.

[dim]

Pinging, since it has come up again on the FreeBSD mailing lists, now this warning is emitted thousands of times. Adding Ed as a reviewer.

[dim]

In D6379#95991, @dblaikie wrote:

Should we add that filter elsewhere rather than here? (eg: we shouldn't be adding such non-text sections into the Sections map if they're not part of teh DWARF anyway, right?)

Maybe, but I don't know the code around this part well enough to make that fix. The above fix is a small one-liner, and solves the problem in a nicely minimal fashion.

& this could use a test case as well.

I'll add one.

[probinson]

The DWARF2 restriction looks like it is related to non-contiguous code ranges, which were added in DWARF 3. So the warning should apply to code sections only. In that respect, functionally I think it's okay.
The diff needs to be refreshed as the surrounding code has changed in the interim, and it still needs a test.

[dim]

Actually I see that we also get this warning on our .section ".note.GNU-stack","",@progbits directives, which are in most of our hand-written .S files. Since the type of that section is progbits, this fix won't work for it.

However, interestingly, clang also outputs these same .note.GNU-stack directives in .s output for compiled files, but it does *not* warn on those, even when using -gdwarf-2. So how is it avoiding them in that case?

[probinson]

In D6379#852974, @dim wrote:

Actually I see that we also get this warning on our .section ".note.GNU-stack","",@progbits directives, which are in most of our hand-written .S files. Since the type of that section is progbits, this fix won't work for it.

However, interestingly, clang also outputs these same .note.GNU-stack directives in .s output for compiled files, but it does *not* warn on those, even when using -gdwarf-2. So how is it avoiding them in that case?

Because when clang compiles a C/C++ source, it generates all the necessary debug-info directives and then runs the assembler phase without -g. The path you are modifying is for running the assembler phase with -g.
If you do clang -S foo.c and clang -gdwarf-2 foo.s you should see the warning.

[nickdesaulniers]

Hello, we're hitting this issue when assembling the Linux kernel with Clang (https://github.com/ClangBuiltLinux/linux/issues/716). I'd be happy to dust this off/rebase and add tests. @dim would you prefer to do it or shall I?

[nickdesaulniers]

Looks like d306c3cec2605ae9100248e6ac097d0325e8aae8 added some tests for r211273 (8b273086176081c4fa803ebb2c09211f0318a73f). cc @olista01 @ostannard

[dim]

In D6379#1934620, @nickdesaulniers wrote:

Hello, we're hitting this issue when assembling the Linux kernel with Clang (https://github.com/ClangBuiltLinux/linux/issues/716). I'd be happy to dust this off/rebase and add tests. @dim would you prefer to do it or shall I?

I had mostly forgotten about it, since we (at least in FreeBSD) switched to DWARF4 by default, so we no longer see this warning. I'm probably too rusty :)

[nickdesaulniers]

So I rebased this, wrote tests that pass, then tried it on my kernel and...it didn't lower the warning count any. So I probably won't be pursing this revision further. Seems that most of our out of line assembly uses .section/.pushsection for progbits (or unspecified, which I guess defaults to progbits), but is all compiled with -Wa,-gdwarf-2.

I'm not really sure what to do; I assume we still want .debug_line info provided by dwarf2, but it's just so common in our out of line assembly to use multiple different sections. @ostannard any thoughts?

[ostannard]

It looks like the issues this is warning about isn't the .debug_line section, but the DW_AT_low/high_pc attributes of the compilation unit DIE. With DWARF3 we can emit a DW_AT_ranges attribute instead, but that didn't exist in DWARF2.

Since this range should only cover PC values corresponding to this compilation unit, maybe we could also limit this warning to sections with SHF_EXECINSTR? Does linux tend to split code across multiple sections, or are the extra sections just used for data?

If that doesn't work, maybe we'll have to add a proper way to suppress individual categories of assembly warnings, like we have for compiler warnings.

[dblaikie]

In D6379#1936534, @ostannard wrote:

It looks like the issues this is warning about isn't the .debug_line section, but the DW_AT_low/high_pc attributes of the compilation unit DIE. With DWARF3 we can emit a DW_AT_ranges attribute instead, but that didn't exist in DWARF2.

Since this range should only cover PC values corresponding to this compilation unit, maybe we could also limit this warning to sections with SHF_EXECINSTR?

Do you have examples where that's not already the case?

Does linux tend to split code across multiple sections, or are the extra sections just used for data?

Yes, in object files - comdat sections are used for inline functions (or in the more extreme case, -ffunction-sections splits every function into its own .text section). Those all get linked into one section by the linker, but at the object file stage there are often multiple .text sections (maybe with unique name suffixes, maybe not (-fno-unique-section-names)).

Try compiling this at -O0:

inline void f1() { }
void f2()   f1(); }

or this with -ffunction-sections:

void f1() { }
void f2() { }

Oh, another way you can get holes in your CU address range even without distinct sections:

void f1() { }
__attribute__((nodebug)) void f1() { }
void f3() { }

lib/MC/MCParser/ELFAsmParser.cpp

const MCSection *ELFSection = getContext().getELFSection(
       SectionName, Type, Flags, Kind, Size, GroupName);
   getStreamer().SwitchSection(ELFSection, Subsection);
 
   if (getContext().getGenDwarfForAssembly()) {
     auto &Sections = getContext().getGenDwarfSectionSyms();
     auto InsertResult = Sections.insert(
         std::make_pair(ELFSection, std::make_pair(nullptr, nullptr)));
     if (InsertResult.second) {
-      if (getContext().getDwarfVersion() <= 2)
+      if (getContext().getDwarfVersion() <= 2 && Type == ELF::SHT_PROGBITS)
         Warning(loc, "DWARF2 only supports one section per compilation unit");
 
       MCSymbol *SectionStartSymbol = getContext().CreateTempSymbol();
       getStreamer().EmitLabel(SectionStartSymbol);
       InsertResult.first->second.first = SectionStartSymbol;
     }
   }