ORC cannot run initializers with non-void signatures #54797

hahnjo · 2022-04-07T15:45:07Z

Consider the following program in C:

#include <stdio.h>

__attribute__((constructor(1001)))
static int constructor(void *ptr) {
  puts("constructor");
  return 0;
}

int main() {
  return 0;
}

or the (stripped down) version in LLVM IR:

@.str = private unnamed_addr constant [12 x i8] c"constructor\00", align 1
@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 1001, void ()* bitcast (i32 (i8*)* @constructor to void ()*), i8* null }]

define internal i32 @constructor(i8* noundef %ptr) {
entry:
  %ptr.addr = alloca i8*, align 8
  store i8* %ptr, i8** %ptr.addr, align 8
  %call = call i32 @puts(i8* noundef getelementptr inbounds ([12 x i8], [12 x i8]* @.str, i64 0, i64 0))
  ret i32 0
}

declare i32 @puts(i8* noundef)

define dso_local i32 @main() {
entry:
  %retval = alloca i32, align 4
  store i32 0, i32* %retval, align 4
  ret i32 0
}

When trying to run with lli --jit-kind=orc constructor.min.ll, there is a segmentation fault because the llvm::Function * and its type are null pointers. This is because the CtorDtorIterator in ExecutionUtils.cpp expects another ConstantExpr in the first operand of a cast, that could be fixed as follows:

diff --git a/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
index ae2d47fb8c5e..cbc27980f57a 100644
--- a/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ExecutionUtils.cpp
@@ -62,7 +62,7 @@ CtorDtorIterator::Element CtorDtorIterator::operator*() const {
       break;
     } else if (ConstantExpr *CE = dyn_cast_or_null<ConstantExpr>(FuncC)) {
       if (CE->isCast())
-        FuncC = dyn_cast_or_null<ConstantExpr>(CE->getOperand(0));
+        FuncC = CE->getOperand(0);
       else
         break;
     } else {

However, then an assertion trips because the code expects to call a function without parameters:

llvm/lib/IR/Instructions.cpp:525: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*> >, const llvm::Twine&): Assertion `(Args.size() == FTy->getNumParams() || (FTy->isVarArg() && Args.size() > FTy->getNumParams())) && "Calling a function with bad signature!"' failed.

This at least matches MCJIT:

llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp:523: virtual llvm::GenericValue llvm::MCJIT::runFunction(llvm::Function*, llvm::ArrayRef<llvm::GenericValue>): Assertion `(FTy->getNumParams() == ArgValues.size() || (FTy->isVarArg() && FTy->getNumParams() <= ArgValues.size())) && "Wrong number of arguments passed into function!"' failed.

@lhames

The text was updated successfully, but these errors were encountered:

llvmbot · 2022-04-07T15:45:35Z

@llvm/issue-subscribers-orcjit

lhames · 2022-04-08T00:18:19Z

Thanks @hahnjo! I've committed a fix in a76209c.

The aim is to move to to the ORC Runtime for running initializers in the future, and the ORC runtime should be able to match the behavior of the dynamic loaders (cast initializer address to void() and call it). Is there any valid use case for non-void initializers though? These feel like something that we might want to warn about in the compiler.

hahnjo · 2022-04-08T06:55:47Z

@lhames as far as I can tell, you only committed the diff that I put above, that I could have done myself. However, as also noted above, this doesn't work if the constructor takes arguments so now it will crash in a different place. Regarding the question if that is valid, it's currently accepted by both Clang and GCC so there may be code using it. In fact, Clang's CUDA support generates constructors taking an argument - I'll have to see today if that can be removed.

hahnjo · 2022-04-08T09:06:16Z

Regarding the question if that is valid, it's currently accepted by both Clang and GCC so there may be code using it.

Yes, if registered via .init_array at least glibc will pass in (argc, argv, envp) to the function. See for example:

#include <stdio.h>

__attribute__((constructor))
static void constructor(int argc, char *argv[], char *envp[]) {
  printf("arguments:\n");
  for (int i = 0; i < argc; i++) {
    printf("  %s\n", argv[i]);
  }
  printf("\nenvironment:\n");
  while (*envp != NULL) {
    printf("  %s\n", *envp);
    envp++;
  }
}

int main() {
  return 0;
}

We don't need that for CUDA, so I'm proposing to remove the argument in https://reviews.llvm.org/D123370.

This works around a bug + missing support in ORC for constructors with non-void signatures (llvm/llvm-project#54797). Also submitted upstream, see https://reviews.llvm.org/D123370

lhames · 2022-04-08T19:39:33Z

Yes, if registered via .init_array at least glibc will pass in (argc, argv, envp) to the function. See for example:

Interesting. I haven't come across this before.

Updating the LLJIT "Generic Platform" support to do this is probably awkward but doable. If you don't need it (and nobody else has asked for it) I'm not sure it's worth the effort for now, but if you'd like to tackle it the right place to look would be in GlobalCtorDtorScraper::operator().

Doing this in the ORC runtime should be more natural, and it's the right long term place to implement this. I think we could leave this bug open to track implementing this in the ORC runtime.

hahnjo · 2022-04-09T10:41:29Z

I've just committed e4903d8 to avoid this situation with CUDA. We'll go with this solution for Cling, so we don't have a requirement for it. I agree that this should be kept open until this is implemented somewhere when / if need arises.

llvmbot · 2022-04-09T11:02:00Z

@llvm/issue-subscribers-clang-codegen

This works around a bug + missing support in ORC for constructors with non-void signatures (llvm/llvm-project#54797). Also submitted upstream, see https://reviews.llvm.org/D123370

This works around a bug + missing support in ORC for constructors with non-void signatures (llvm#54797). Also submitted upstream, see https://reviews.llvm.org/D123370

hahnjo added the orcjit label Apr 7, 2022

lhames self-assigned this Apr 8, 2022

EugeneZelenko added the clang:codegen label Apr 9, 2022

EugeneZelenko removed the clang:codegen label Apr 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ORC cannot run initializers with non-void signatures #54797

ORC cannot run initializers with non-void signatures #54797

hahnjo commented Apr 7, 2022

llvmbot commented Apr 7, 2022

lhames commented Apr 8, 2022

hahnjo commented Apr 8, 2022

hahnjo commented Apr 8, 2022

lhames commented Apr 8, 2022

hahnjo commented Apr 9, 2022

llvmbot commented Apr 9, 2022

ORC cannot run initializers with non-void signatures #54797

ORC cannot run initializers with non-void signatures #54797

Comments

hahnjo commented Apr 7, 2022

llvmbot commented Apr 7, 2022

lhames commented Apr 8, 2022

hahnjo commented Apr 8, 2022

hahnjo commented Apr 8, 2022

lhames commented Apr 8, 2022

hahnjo commented Apr 9, 2022

llvmbot commented Apr 9, 2022