llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.58k stars 11.34k forks source link

`va_arg` fails with `__int128` #20283

Open llvmbot opened 10 years ago

llvmbot commented 10 years ago
Bugzilla Link 19909
Version trunk
OS All
Attachments Demonstrate __int128 va_arg failure
Reporter LLVM Bugzilla Contributor
CC @tstellar

Extended Description

The attached example fails on Clang/LLVM trunk (or 3.4 or Apple 3.4) and passes when compiled with gcc 4.8.2. The code simple tries to pass multiple __int128 after two ints and a pointer. I debugged it a bit and it seems that the overflow_arg_area pointer in va_list is wrong by 8 bytes, which might indicate some alignment problem.

I also checked how __int128 is passed to the function. It seems that Clang is capable of splitting the __int128 where lower part is passed in a register and upper part on the stack. GCC does not split __int128 if it does not have 2 registers available. This might explain the wrong 8 bytes offset for the overflow_arg_area as va_start does not adjust for the split register.

tstellar commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#42439

tgross35 commented 1 year ago

I think this is related to https://github.com/llvm/llvm-project/issues/41784

shafik commented 1 year ago

code: https://godbolt.org/z/bxrdrMKne

#include <stdarg.h>
#include <stdio.h>
#include <assert.h>
#include <string.h>

int si;
void *sp;

double foo(va_list ap) {
  int x = va_arg(ap, int);
  double y = *va_arg(ap, double *);

  __int128 psi = va_arg(ap, __int128);
  assert(psi == (__int128)&si);
  __int128 psv = va_arg(ap, __int128);
  assert(psv == (__int128)&si);
  __int128 psiB = va_arg(ap, __int128);
  assert(psiB == (__int128)&si);
  __int128 psiI = va_arg(ap, __int128);
  assert(psiI == (__int128)&si);
  __int128 psi0 = va_arg(ap, __int128);
  assert(psi0 == (__int128)&si);
  __int128 psi1 = va_arg(ap, __int128);
  assert(psi1 == (__int128)&si);
  void **ppsv = va_arg(ap, void **);
  assert(*ppsv == &si);
  void **ppsv2 = va_arg(ap, void **);
  assert(*ppsv2== &si);
  return x + y;
}

double bar(int last, ...) {
  va_list ap;
  double d;

  va_start(ap, last);
  d = foo(ap);
  va_end(ap);

  return d;
}
int main(void) {
  sp = &si;
  __int128 psv = (__int128)&si;
  __int128 psi = (__int128)&si;
  __int128 psiB = psv;
  __int128 psiI = psv;
  __int128 psi0 = psv;
  __int128 psi1 = psv;
  void **ppsv = &sp;
  double a = 7.0;
  si = 10;
  double b = bar(0, 1, &a, psi, psv, psiB, psiI, psi0, psi1, ppsv, ppsv);

  if (b != 8.0) {
    /* While floating-point comparisions are not safe in general, small integers are OK */
    printf("FAILED (result = %g but expect 8.0)\n", b);
  } else {
    printf("SUCCESS\n");
  }

  return 0;
}

clang fails with the following output:

output.s: /app/example.cpp:16: double foo(__va_list_tag *): Assertion `psv == (__int128)&si' failed.
Program terminated with signal: SIGSEGV
shafik commented 1 year ago

CC @AaronBallman

tgross35 commented 1 year ago

I believe that the patch in https://reviews.llvm.org/D86310 will fix this once it resolves the register spilling

tgross35 commented 5 months ago

If the fix for this is easy after https://reviews.llvm.org/D86310 landed, it may be a good candidate for 18.2 to keep all i128 ABI changes within the 18.x series.

cc @nikic since you were involved with the other i128 fixes

Endilll commented 3 weeks ago

CC @AaronBallman I wonder if _BitInt(128) has a similar issue.

llvmbot commented 3 weeks ago

@llvm/issue-subscribers-clang-codegen

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [19909](https://llvm.org/bz19909) | | Version | trunk | | OS | All | | Attachments | [Demonstrate __int128 va_arg failure](https://user-images.githubusercontent.com/60944935/143749535-9bd0b9b4-75a6-4299-9717-57a7dff7c1d9.gz) | | Reporter | LLVM Bugzilla Contributor | | CC | @tstellar | ## Extended Description The attached example fails on Clang/LLVM trunk (or 3.4 or Apple 3.4) and passes when compiled with gcc 4.8.2. The code simple tries to pass multiple `__int128` after two ints and a pointer. I debugged it a bit and it seems that the `overflow_arg_area` pointer in `va_list` is wrong by 8 bytes, which might indicate some alignment problem. I also checked how `__int128` is passed to the function. It seems that Clang is capable of splitting the `__int128` where lower part is passed in a register and upper part on the stack. GCC does not split `__int128` if it does not have 2 registers available. This might explain the wrong 8 bytes offset for the `overflow_arg_area` as `va_start` does not adjust for the split register.
AaronBallman commented 3 weeks ago

I can confirm there's an issue with __int128; when trying with _BitInt(128), it succeeds: https://godbolt.org/z/TnEsE7Pse