[PATCH] Insert poisoned paddings between fields in C++ classes.

[Kostya Serebryany]

Addressed all comments except for one

================
Comment at: include/clang/AST/Decl.h:3266
@@ -3265,1 +3265,3 @@
 
+  /// MayInsertExtraPadding -- Whether we are allowed to insert extra padding
+  /// between fields. These padding are added to help AddressSanitizer detect
----------------
rsmith wrote:
> In new code, please use `\brief` instead of repeating the function name.
done

================
Comment at: include/clang/AST/Decl.h:3269
@@ +3268,3 @@
+  /// intra-object-overflow bugs.
+  bool MayInsertExtraPadding(bool EmitRemark = false) const;
+
----------------
rsmith wrote:
> Please use a leading lowercase letter for this function name for consistency with the rest of the class.
done

================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:49-51
@@ -48,1 +48,5 @@
 
+def remark_sanitize_address_insert_extra_padding : Remark<
+    "MayInsertExtraPadding %0: %1">, BackendInfo,
+    InGroup<SanitizeAddressRemarks>;
+
----------------
rsmith wrote:
> Remarks are end-user-visible, so should be a bit more friendly than this. Also, this diagnostic should use "%select{list|of|strings}1" rather than putting user-visible strings in the code in Decl.cpp. Something like:
> 
>   "-fsanitize-address-field-padding ignored for %0 because it %select{is packed|is a union|is trivially copyable|has a trivial destructor|is standard layout|is in a blacklisted file|is blacklisted}1"
> 
> ... would seem reasonable. (I don't think it's worth remarking once per class if the reason we're not sanitizing is because the current language is C; in that case, we should warn once for the entire compilation.)
Done except for "we should warn once for the entire compilation for C"
What is the kosher way to achieve that in clang? 
(I assume that a function-scope static boolean is not kosher)

================
Comment at: lib/AST/Decl.cpp:3633
@@ +3632,3 @@
+  // We may be able to relax some of these requirements.
+  const char *ReasonToReject = 0;
+  if (!CXXRD)
----------------
rsmith wrote:
> Please use `nullptr` rather than `0`.
Var changed to int.

================
Comment at: lib/AST/Decl.cpp:3642
@@ +3641,3 @@
+    ReasonToReject = "trivially copyable";
+  else if (CXXRD->hasSimpleDestructor())
+    ReasonToReject = "has simple DTOR";
----------------
rsmith wrote:
> `hasSimpleDestructor` still does not seem right; it's a notion internal to how we manage implicit declaration and definition for destructors and shouldn't be used for anything else. Does `hasTrivialDestructor` work for you?
Yes, this seems to work,

================
Comment at: lib/AST/Decl.cpp:3648
@@ +3647,3 @@
+    ReasonToReject = "blacklisted by src";
+  else if (II && SCL->inSection("field-padding-type", II->getName()))
+    ReasonToReject = "blacklisted by type";
----------------
rsmith wrote:
> Blacklisting by identifier doesn't seem ideal; there should probably be some way of specifying a namespace in the blacklist file.
done (using getQualifiedNameAsString())

http://reviews.llvm.org/D5687