kernigh added inline comments.
================ Comment at: clang/lib/CodeGen/TargetInfo.cpp:4723 + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || + Ty->isAggregateType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; ---------------- kernigh wrote: > efriedma wrote: > > I suspect this code doesn't handle C++ member pointers correctly. But > > maybe we can leave that for a followup. > > > > Could we simplify this to `bool isInt = !Ty->isFloatingType();`? > Yes, C++ member pointers are broken. I declared a struct animal, tried > `typedef void (animal::*noise)(); ... va_arg(ap, noise)`, and it segfaulted. > In the disassembly, I saw 2 problems: (1) va_arg was looking at the > floating-point registers, and (2) va_arg was not indirecting through a > pointer. The caller had passed a pointer to a `noise` in a general-purpose > register. > > I'm not sure about `bool isInt = !Ty->isFloatingType();`, because I haven't > checked whether it would break other types. `bool isInt = !Ty->isFloatingType();` is working for me. I'm doing more checks; if it continues to work, I will update this diff. Another change in `bool isIndirect = Ty->isAggregateType();` to instead call `isAggregateTypeForABI(Ty)` is fixing va_arg of a C++ member function pointer. I may want to put this change in the same diff because I will be running both changes together. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90329/new/ https://reviews.llvm.org/D90329 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
