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

[Gui Andrade via Phabricator via llvm-commits]

guiand marked an inline comment as done.
guiand added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2082
+  const Type *RetTyPtr = RetTy.getTypePtr();
+  if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() &&
+      RetAI.getKind() != ABIArgInfo::Indirect) {
----------------
rsmith wrote:
> Other types that we should think about from a padding perspective:
> 
>  * `nullptr_t` (which has the same size and alignment as `void*` but is all padding)
>  * 80-bit `long double` and `_Complex long double` (I don't know whether we guarantee to initialize the 6 padding bytes)
>  * member pointers might contain padding under some ABI rules -- under the MS ABI, you get a struct containing N pointers followed by M ints, which could have 4 bytes of tail padding on 64-bit targets
>  * vector types with tail padding (eg, a vector of 3 `char`s is sometimes passed as an `i32` with one byte `undef`)
>  * matrix types (presumably the same concerns as for vector types apply here)
>  * maybe Obj-C object types?
>  * `_ExtInt` types (eg, returning an `_ExtInt(65)` initialized to 0 produces an `{i64, i64}` containing 7 bytes of `undef`)
> 
> It would be safer to list exactly those types for which we know this assumption is correct rather than assuming that it's correct by default -- I think more than half of the `Type` subclasses for concrete canonical types can have undef bits in their IR representation.
This is a good point, and I think there was some nuance that I had previously included in determining this for records (since removed) that I didn't think to include in CGCall.cpp.

I think one particular check, `llvm::DataLayout::typeSizeEqualsStoreSize`, handles a lot of these cases:
- `long double`
- oddly-sized vectors
- `_ExtInt` types

I don't know if the `matrix` type has internal padding (like structs) but if not it would cover that as well.  
I believe that the struct with padding wouldn't be an issue as we're excluding record types here.  
And I'd have to take a closer look at `nullptr_t` to see how it behaves here.

~~~

But if I'd like to build an exhaustive list of types I think will work correctly with a check like this:
- scalars
- floating point numbers
- pointers

I think I'd need also need an inner `typeSizeEqualsStoreSize` check on the base type of vectors/complex numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678