[Clang-repl] Assertion "Narrow integer argument must have a valid extension type." failed.

[JonPsson1]

With the recent strengthening of the handling of extensions of narrow integer arguments (commit 1412022), it is now possible to detect cases of missing argument extensions, although there are still some issues that keeps this disabled by default. Many tests under Interpreter/ fails, for example:

cat ~/llvm-project/clang/test/Interpreter/assigment-with-implicit-ctor.cpp | ~/llvm-project/build/bin/clang-repl -argext-abi-check
good
SystemZISelLowering.cpp:9861: void llvm::SystemZTargetLowering::verifyNarrowIntegerArgs(const llvm::SmallVectorImpl<llvm::ISD::OutputArg>&, bool) const: Assertion `(VT != MVT::i32 || (Flags.isSExt() || Flags.isZExt() || Flags.isNoExt())) && "Narrow integer argument must have a valid extension type."' failed.

What this means is that there is a narrow (<=32 bit) integer argument that is neither sign or zero extended, and is also not marked as noext, which should be done for "struct-in-reg". So this may or may not be a "bug" (missing sign/zero extension), or it could be an argument that should be marked as noext. See https://llvm.org/docs/LangRef.html#parameter-attributes.

CCing some folks with the hope of help with this, thanks: @vgvassilev @hahnjo @vitalybuka @nico @weliveindetail @nikic

[JonPsson1]

With #109699, I get

cat ~/llvm-project/clang/test/Interpreter/assigment-with-implicit-ctor.cpp | ~/llvm-project/build/bin/clang-repl -argext-abi-check
good
ERROR: Missing extension attribute of returned value from function:
i32 @atexit(ptr)

My guess is that the returned value from @atexit needs sign or zero extension...

CCing some folks with the hope of help with this, thanks: @vgvassilev @hahnjo @vitalybuka @nico @weliveindetail @nikic

[vgvassilev]

Thanks for submitting a bug report and debugging it too. I am not sure what is going on here I think your guess is probably the best clue I have, too...

[nikic]

Presumably the problem is

llvm-project/clang/lib/CodeGen/CGDeclCXX.cpp

Lines 358 to 359 in cc7b24a

	CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(),
	/*Local=*/true);

Which creates the function without applying necessary ABI attributes.

I'd hope that clang codegen has some method to generate these kinds of runtime functions with appropriate attributes without requiring special casing for each one, but I wouldn't know what it is. Maybe the clang codegen maintainers (@rjmccall @efriedma-quic) have some pointers.

[JonPsson1]

My understanding is that you really have to do this specifically for each one as you have to specify if the integer is signed or unsigned, which there is otherwise no way to know.

There is the TLI->getExtAttrForI32Param(Triple, bool Signed), which seems to be used also outside BuildLibCalls, e.g. by the AddressSanitizer. This is however merely the next step after you know the signedness of the argument to let the target further specify what (if any) extension attribute should actually be added. Not sure however if that is required outside of libcalls, though, but it seems it would be reasonable.

[efriedma-quic]

CodeGenModule::CreateRuntimeFunction doesn't have enough information to mark up functions correctly, given its current signature. I don't think we currently have an alternative. We could add one pretty easily, though; we just a version that takes a clang::FunctionType instead of an llvm::FunctionType.

[rjmccall]

Most runtime functions are specified in terms of the C ABI, so yeah, we should probably change up how we emit runtime calls in general to go through the standard call-emission functions. Maybe we can preserve a fast path for common runtime calls by designing a system for caching the runtime function and just make it also record when all the arguments use simple direct argument passing without any attributes.

[JonPsson1]

The improvement to dump the function signature for cases of missing extension attributes has now been committed.

[vgvassilev]

Can you link the commit here for future reference?

[JonPsson1]

Can you link the commit here for future reference?

f9fbfc5

[JonPsson1]

ping...

[nikic]

@JonPsson1 Anything in particular you are waiting for here? @efriedma-quic's suggested approach to fixing this sounds reasonable to me.