[PATCH] D138511: [CodeGen][AArch64] Fix AArch64ABIInfo::EmitAAPCSVAArg crash with empty record type in variadic arg

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D138511#3954143 <https://reviews.llvm.org/D138511#3954143>, @efriedma wrote:

> It looks like AArch64ABIInfo::classifyArgumentType does a slightly more complicated check for "empty"; does that matter here?

Thanks for your comments @efriedma. Good catch!  Current patch has a issue in this case(C++ mode):

clang -Xclang -no-opaque-pointers -target aarch64-linux-gnu -emit-llvm -S ./va_arg.cpp

  typedef __builtin_va_list va_list;
  #define va_start(ap, param) __builtin_va_start(ap, param)
  #define va_end(ap)          __builtin_va_end(ap)
  #define va_arg(ap, type)    __builtin_va_arg(ap, type)
  
  va_list the_list;
  typedef struct {} empty;
  empty empty_record_test(void) {
    return va_arg(the_list, empty);
  }

When use `isEmptyRecord` directly to check empty record, the generated IR is:

  define dso_local void @_Z17empty_record_testv() #0 {
  entry:
    %0 = load i8*, i8** getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 0), align 8
    %1 = bitcast i8* %0 to %struct.empty*
    ret void
  }

When use `AArch64ABIInfo::classifyArgumentType(...).isIgnore()` to check empty record, the generated IR is:

  define dso_local void @_Z17empty_record_testv() #0 {
  entry:
    %gr_offs = load i32, i32* getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 3), align 8
    %0 = icmp sge i32 %gr_offs, 0
    br i1 %0, label %vaarg.on_stack, label %vaarg.maybe_reg
  
  vaarg.maybe_reg:                                  ; preds = %entry
    %new_reg_offs = add i32 %gr_offs, 8
    store i32 %new_reg_offs, i32* getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 3), align 8
    %inreg = icmp sle i32 %new_reg_offs, 0
    br i1 %inreg, label %vaarg.in_reg, label %vaarg.on_stack
  
  vaarg.in_reg:                                     ; preds = %vaarg.maybe_reg
    %reg_top = load i8*, i8** getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 1), align 8
    %1 = getelementptr inbounds i8, i8* %reg_top, i32 %gr_offs
    %2 = bitcast i8* %1 to %struct.empty2*
    br label %vaarg.end
  
  vaarg.on_stack:                                   ; preds = %vaarg.maybe_reg, %entry
    %stack = load i8*, i8** getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 0), align 8
    %new_stack = getelementptr inbounds i8, i8* %stack, i64 8
    store i8* %new_stack, i8** getelementptr inbounds (%"struct.std::__va_list", %"struct.std::__va_list"* @the_list, i32 0, i32 0), align 8
    %3 = bitcast i8* %stack to %struct.empty2*
    br label %vaarg.end
  
  vaarg.end:                                        ; preds = %vaarg.on_stack, %vaarg.in_reg
    %vaargs.addr = phi %struct.empty2* [ %2, %vaarg.in_reg ], [ %3, %vaarg.on_stack ]
    ret void
  }

As far as I know,  according to the comments of the `AArch64ABIInfo::classifyArgumentType(...)`, "Empty records are always ignored on Darwin, but actually passed in C++ mode elsewhere for GNU compatibility." and "GNU C mode. The only argument that gets ignored is an empty one with size 0.", I think we should ignore arg only if DarwinPCS ABI and empty record in C mode. What do you all think about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138511