[PATCH] D139395: Add CFI integer types normalization

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In D139395#3990772 <https://reviews.llvm.org/D139395#3990772>, @rcvalle wrote:

> I elaborated on the reasons why not use a generalized encoding in the design document in the tracking issue https://github.com/rust-lang/rust/issues/89653. The tl;dr; is that it will result in less comprehensive protection by either using a generalized encoding for all C and C++ -compiled code or across the FFI boundary, and will degrade the security of the program when linking foreign Rust-compiled code into a program written in C or C++ because the program previously used a more comprehensive encoding for all its compiled code, not fixing the issue described in the design document and the RFC https://github.com/rcvalle/rfcs/blob/improve-c-types-for-cross-language-cfi/text/0000-improve-c-types-for-cross-language-cfi.md#appendix.

Ack.



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:2952
+  // u<length>u<type size>
+  if (NormalizeIntegers && T->isInteger()) {
+    if (T->isSignedInteger()) {
----------------
rcvalle wrote:
> pcc wrote:
> > `isInteger()` will return true for enums, but only if they are complete. This would mean that code such as
> > ```
> > void (*f)(enum E *e);
> > 
> > void g() {
> >   f(0);
> > }
> > ```
> > would use a different encoding to call `f` depending on whether the TU completes the enum `E`, if pointee types are considered part of the encoding.
> Isn't `isIntegerType()` that does that? `isInteger()` definition is:
> 
> ```
>   bool isInteger() const {
>     return getKind() >= Bool && getKind() <= Int128;
>   }
> ```
Ah yes, sorry, somehow I read this as a call to `isIntegerType()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395