Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] fix emitvaarg when struct is null #72624

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jolyon0202
Copy link

Fix bug when emit null struct with attribute aligned(16) and ICE of debugbuild.

Fix bug when emit null struct with attribute aligned(16) and ICE of debugbuild.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:codegen labels Nov 17, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 17, 2023

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Jolyon (Jolyon0202)

Changes

Fix bug when emit null struct with attribute aligned(16) and ICE of debugbuild.


Full diff: https://github.com/llvm/llvm-project/pull/72624.diff

1 Files Affected:

  • (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+2-2)
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index be5145daa00b7f5..49dff102b7d554a 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -526,7 +526,7 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(Address VAListAddr, QualType Ty,
   llvm::Type *BaseTy = CGF.ConvertType(Ty);
   if (IsIndirect)
     BaseTy = llvm::PointerType::getUnqual(BaseTy);
-  else if (AI.getCoerceToType())
+  else if (AI.canHaveCoerceToType() && AI.getCoerceToType())
     BaseTy = AI.getCoerceToType();
 
   unsigned NumRegs = 1;
@@ -594,7 +594,7 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(Address VAListAddr, QualType Ty,
   // Integer arguments may need to correct register alignment (for example a
   // "struct { __int128 a; };" gets passed in x_2N, x_{2N+1}). In this case we
   // align __gr_offs to calculate the potential address.
-  if (!IsFPR && !IsIndirect && TyAlign.getQuantity() > 8) {
+  if (!IsFPR && !IsIndirect && TyAlign.getQuantity() > 8 && !TySize.isZero()) {
     int Align = TyAlign.getQuantity();
 
     reg_offs = CGF.Builder.CreateAdd(

@Jolyon0202
Copy link
Author

demo:

#include <stdio.h>
extern void *memset (void *__s, int __c, size_t __n);
typedef __builtin_va_list va_list;

struct S94 {
  struct __attribute__((aligned(16))) {
  } a;
};

struct S94 s94;

void check94va(int z, ...) {
  va_list ap;
  __builtin_va_start(ap, z);
  struct S94 arg1 =  __builtin_va_arg(ap, struct S94);
  long long int tmp = __builtin_va_arg(ap, long long);
  printf("result = %lld\n", tmp);
  if (tmp != 2LL) {
    printf("Fails!!!!\n");
  }
  __builtin_va_end(ap);
}

int main(void) {
  memset(&s94, '\0', sizeof(s94));
  printf("sizeof(s94) = %ld\n", sizeof(s94));
  check94va(1, s94, 2LL);
  return 0;
}

found two bug need to fix:
1 If the alignment of S94(null struct) is less than 16, the result is correct.
2 ICE of AI.getCoerceToType() in debugbuild.(AI can not getcoerceTotype when it's null)

@Jolyon0202
Copy link
Author

@efriedma-quic

@efriedma-quic
Copy link
Collaborator

Please fix the pull request description to say this is aarch64-specific.

Missing regression test in clang/test/CodeGen/

How does this interact with #72197?

@s-barannikov s-barannikov requested review from DavidSpickett and TNorthover and removed request for DavidSpickett December 19, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants