[clang][codegen] Fix possible crash when setting TBAA metadata on FP math libcalls

[MacDue]

There's currently no code path that can reach this crash, but:

Instruction *Inst = cast<llvm::Instruction>(Call.getScalarVal());

fails if the call returns void. This could happen if a builtin for something like void sincos(double, double*, double*) is added to clang.

Instead, use the llvm::CallBase returned from EmitCall() to set the TBAA metadata, which should exist no matter the return type.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Benjamin Maxwell (MacDue)

Changes

There's currently no code path that can reach this crash, but:

Instruction *Inst = cast<llvm::Instruction>(Call.getScalarVal());

fails if the call returns void. This could happen if a builtin for something like void sincos(double, double*, double*) is added to clang.

Instead, use the llvm::CallBase returned from EmitCall() to set the TBAA metadata, which should exist no matter the return type.

---

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

1 Files Affected:

• (modified) clang/lib/CodeGen/CGBuiltin.cpp (+4-3)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 27abeba92999b3..d4c7eea3d20b24 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -690,8 +690,10 @@ static RValue emitLibraryCall(CodeGenFunction &CGF, const FunctionDecl *FD,
                               const CallExpr *E, llvm::Constant *calleeValue) {
   CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
   CGCallee callee = CGCallee::forDirect(calleeValue, GlobalDecl(FD));
+  llvm::CallBase *callOrInvoke = nullptr;
   RValue Call =
-      CGF.EmitCall(E->getCallee()->getType(), callee, E, ReturnValueSlot());
+      CGF.EmitCall(E->getCallee()->getType(), callee, E, ReturnValueSlot(),
+                   /*Chain=*/nullptr, &callOrInvoke);
 
   if (unsigned BuiltinID = FD->getBuiltinID()) {
     // Check whether a FP math builtin function, such as BI__builtin_expf
@@ -705,8 +707,7 @@ static RValue emitLibraryCall(CodeGenFunction &CGF, const FunctionDecl *FD,
       // Emit "int" TBAA metadata on FP math libcalls.
       clang::QualType IntTy = Context.IntTy;
       TBAAAccessInfo TBAAInfo = CGF.CGM.getTBAAAccessInfo(IntTy);
-      Instruction *Inst = cast<llvm::Instruction>(Call.getScalarVal());
-      CGF.CGM.DecorateInstructionWithTBAA(Inst, TBAAInfo);
+      CGF.CGM.DecorateInstructionWithTBAA(callOrInvoke, TBAAInfo);
     }
   }
   return Call;

[vfdff]

LGTM, thank you for your improvement

[mstorsjo]

I've bisected breakage, on mingw x86 platforms relating to double vs long double routines, down to this commit.

I'll follow up with more details when I manage to pinpoint what changes and where, due to this change.

[MacDue]

Thanks for letting me know, feel free to revert this change if it's a non-trivial breakage.

[mstorsjo]

I managed to reduce the breakage to the following snippet:

long double powl(long double a, long double b);
long double a() { return powl(2.0L, 2.0L); }

Compiled like this:

$ clang -target x86_64-w64-mingw32 -S -O2 -o out.s repro.c

The output between before and after this change differs like this:

--- out-good.s  2024-09-16 13:45:09.505125890 +0300
+++ out-bad.s   2024-09-16 13:45:09.533125294 +0300
@@ -10,12 +10,7 @@
        .scl    2;
        .type   32;
        .endef
-       .section        .rdata,"dr"
-       .p2align        2, 0x0                          # -- Begin function a
-.LCPI0_0:
-       .long   0x40000000                      # float 2
-       .text
-       .globl  a
+       .globl  a                               # -- Begin function a
        .p2align        4, 0x90
 a:                                      # @a
 .seh_proc a
@@ -26,16 +21,10 @@
        .seh_stackalloc 80
        .seh_endprologue
        movq    %rcx, %rsi
-       flds    .LCPI0_0(%rip)
-       fld     %st(0)
-       fstpt   48(%rsp)
-       fstpt   32(%rsp)
        leaq    64(%rsp), %rcx
        leaq    48(%rsp), %rdx
        leaq    32(%rsp), %r8
        callq   powl
-       fldt    64(%rsp)
-       fstpt   (%rsi)
        movq    %rsi, %rax
        addq    $80, %rsp
        popq    %rsi

I'll push a revert shortly.

[MacDue]

Thanks for the reduction! I tracked this down to the "int" TBAA metadata being added to calls with indirect arguments (with seems broken even without this change).

I've created a possible fix here: #108853 (comment).