[MC] report error instead of assertion for non-zero initializer in .bss section

[weimingz]

Summary

User may initialize a variable with non-zero value and put it into .bss section by mistake.

E.g. : int a __attribute__((section(".bss"))) = 2; // mistakenly initialized to 2

This patch converts an assertion to error report for better user experience.

[rnk]

This seems like an OK diagnostic for now in LLVM, but eventually we will need one in clang.

lib/MC/MCAssembler.cpp

789–790 ↗

(On Diff #10591)

This creates a StringRef that points to a destroyed std::string.

[• rafael]

lib/MC/MCAssembler.cpp

788 ↗

(On Diff #10600)

You can probably use a dyn_cast.

791 ↗

(On Diff #10600)

How about just having two report_fatal_error calls? The ELF one could be

report_fatal_error("non-zero initializer found in " + Section->getSectionName())

and the non-elf one

report_fatal_error("non-zero initializer found in an virtual section");

792 ↗

(On Diff #10600)

report_fatal_error doesn't return, so you don't need this.

[weimingz]

Hi Rafael,

Thanks for reviewing. The new patch is based on your suggestion.

[• rafael]

lib/MC/MCAssembler.cpp

787 ↗

(On Diff #10705)

Sorry, what I mean is that you can use the dyn_cast in here. It will return a MCSectionELF or null. So instead of

if (check)

cast

you have

if (auto *ELFSection = dyn_cast...)

Use ELFSection

[weimingz]

Hi Rafael,

Sorry I didn't get it. Now it makes sense. :D

[• rafael]

LGTM with a nit.

lib/MC/MCAssembler.cpp

787 ↗

(On Diff #10708)

nit:

use 'auto *' to make it clear that it is a pointer.

[weimingz]

In D4199#3, @rnk wrote:

This seems like an OK diagnostic for now in LLVM, but eventually we will need one in clang.

It's good have a check in clang with more detailed info. (line no. etc).

In D4199#13, @rafael wrote:

LGTM with a nit.

Thanks! I'll commit with the change.

lib/MC/MCAssembler.cpp

789–790 ↗

(On Diff #10591)

Thanks. Will fix.

[weimingz]

Closed by commit rL211455 (authored by @weimingz).

llvm/trunk/lib/MC/MCAssembler.cpp

 #include "llvm/MC/MCSection.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/MC/MCSectionELF.h"
 #include <tuple>
 using namespace llvm;
 
 #define DEBUG_TYPE "assembler"
 
 namespace {
 namespace stats {
 STATISTIC(EmittedFragments, "Number of emitted assembler fragments - total");
for (MCSectionData::const_iterator it = SD->begin(),
       case MCFragment::FT_Data: {
         // Check that we aren't trying to write a non-zero contents (or fixups)
         // into a virtual section. This is to support clients which use standard
         // directives to fill the contents of virtual sections.
         const MCDataFragment &DF = cast<MCDataFragment>(*it);
         assert(DF.fixup_begin() == DF.fixup_end() &&
                "Cannot have fixups in virtual section!");
         for (unsigned i = 0, e = DF.getContents().size(); i != e; ++i)
-          assert(DF.getContents()[i] == 0 &&
-                 "Invalid data value for virtual section!");
+          if (DF.getContents()[i]) {
+            if (auto *ELFSec = dyn_cast<const MCSectionELF>(&SD->getSection()))
+              report_fatal_error("non-zero initializer found in section '" +
+                  ELFSec->getSectionName() + "'");
+            else
+              report_fatal_error("non-zero initializer found in virtual section");
+          }
         break;
       }
       case MCFragment::FT_Align:
         // Check that we aren't trying to write a non-zero value into a virtual
         // section.
         assert((cast<MCAlignFragment>(it)->getValueSize() == 0 ||
                 cast<MCAlignFragment>(it)->getValue() == 0) &&
                "Invalid align in virtual section!");

llvm/trunk/test/MC/ELF/ARM/bss-non-zero-value.s

+// RUN: not llvm-mc -filetype=obj -triple arm-linux-gnu %s -o %t 2>%t.out
+// RUN: FileCheck --input-file=%t.out %s
+// CHECK: non-zero initializer found in section '.bss'
+  .bss
+  .globl  a
+  .align  2
+a:
+  .long 1
+  .size a, 4