[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

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

[• rafael]

lib/MC/MCAssembler.cpp

788

You can probably use a dyn_cast.

791

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

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

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

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

Thanks. Will fix.

[weimingz]

Closed by commit rL211455 (authored by @weimingz).

lib/MC/MCAssembler.cpp

 #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;
 

         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 (SD->getSection().getVariant() == MCSection::SV_ELF)
+              report_fatal_error("non-zero initializer found in section '" +
+                  dyn_cast<const MCSectionELF>
+                  (&(SD->getSection()))->getSectionName() + "'");
+            else
+              report_fatal_error("non-zero initializer found in virtual section");
+
+          }
         break;
       }
       case MCFragment::FT_Align:

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