[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

It's not entirely clear to me what `partialinit` means. Does it mean that some bytes of the parameter might be undef / poison? Or does it mean that some bytes of the parameter that (for a struct parameter or array thereof) correspond to a struct member might be undef / poison? (Eg, if I pass a `{ i8, i32 }` to a function, do we need to explicitly say that the three padding bytes are not initialized?)

I agree with @efriedma that a positive property, something like "these byte / bit ranges within the parameter are not undef", would probably fit better, and are likely usable by optimization passes as well as by msan. (Can we put metadata on parameters yet? This would seem well-suited to being modeled as metadata.)



================
Comment at: clang/include/clang/AST/Type.h:2139-2141
+  /// Check if this type has only two possible values, and so may be lowered to
+  /// a bool.
+  bool hasBooleanRepresentation() const;
----------------
This seems like a CodeGen-specific concern; I'm not sure this makes sense as a query on the Type.


================
Comment at: clang/lib/AST/Type.cpp:2752-2753
+
+  if (const EnumType *ET = getAs<EnumType>())
+    return ET->getDecl()->getIntegerType()->isBooleanType();
+
----------------
Under `-fstrict-enums` in C++, `enum E { a = 0, b = 1 };` has only two distinct valid values. Should we consider that case?


================
Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:679
 
+void CGRecordLowering::determineMemberPartialInit() {
+  auto hasTailPadding = [&](QualType FieldTypePtr) {
----------------
We already have support for C++'s `__has_unique_object_representations` (`ASTContext::hasUniqueObjectRepresentations`), which does something very similar to this. Do we need both? (Are there cases where this is intentionally diverging from `__has_unique_object_representations`?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678