[PATCH] D11221: Fix crash when using __attribute__((section (".bss.name"))) incorrectly

Richard Smith richard at metafoo.co.uk
Wed Jul 15 13:56:13 PDT 2015


rsmith added a subscriber: rsmith.
rsmith added a comment.

High-level comments:

1. This is wrong for C++. A non-zero initializer there is not a problem unless we are required to perform constant initialization for the variable. In any other case, we should emit the variable to the specified section and emit a dynamic initializer for it. (We'll need to check that globalopt doesn't optimize away that initialization and give the variable a non-zero initializer.)
2. It seems unnecessarily kind to allow any initializer at all on these variables. If we want to reject these cases, why not just reject any case where the variable has a specified initializer? (I think it's less reasonable to do this in C++, but whatever we do, making the constant evaluator smarter should not result in us rejecting more code.)
3. Sema is not in a position to decide for itself which constants / types will correspond to all-zero-bits (and for instance the MS ABI and Itanium ABI will disagree on whether some member pointer types are all-zero-bits). It should ask the CXXABI object to determine that on its behalf.
4. We should not be hard-coding knowledge of a target-specific ".bss" section into Sema. We should determine if the section has known properties by asking the target.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2180-2183
@@ -2179,1 +2179,6 @@
+def err_attribute_section_bss_nonzero : Error<
+  "variable placed in BSS section (\"%0\") must be zero-initialized, but "
+  "\'%1\' is initialized with non-zero value">;
+def err_attribute_section_bss_function : Error<
+  "functions cannot be placed in BSS section (\"%0\")">;
 
----------------
No need for `\` before `'` here.


Repository:
  rL LLVM

http://reviews.llvm.org/D11221







More information about the cfe-commits mailing list