llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Takuto Ikuta (atetubou) <details> <summary>Changes</summary> … inherit dllimport When compiling C++ code that calls standard library math functions (e.g. `std::hypot(float, float)`), standard library headers (such as libc++) typically implement these via compiler builtins (`__builtin_hypotf`). During CodeGen, `CodeGenModule::getBuiltinLibFunction` derives the library symbol name (`"hypotf"`) and emits an IR declaration. Previously, `getBuiltinLibFunction` created this IR declaration directly from the builtin's `FunctionDecl`. Consequently, attributes declared on the real standard library function (specifically `__declspec(dllimport)` in system headers like `<math.h>`) were not attached to the IR symbol. On Windows MSVC targets (/MD with Clang Modules), this caused a subtle symbol-level non-determinism (generating raw `hypotf` instead of `__imp_hypotf`) between Linux cross-compilation and Windows native compilation. This is believed to be triggered by the interplay of two mechanisms: 1. Clang Modules Lazy Deserialization: In non-module builds, textual `#include <math.h>` eagerly parses the library declaration into the top-level AST lookup table. In Clang Modules (`-fmodules`), declarations in PCM cache files are lazily deserialized by `ASTReader` only when looked up by name. Because the builtin call referenced `"__builtin_hypotf"`, `"hypotf"` was not eagerly deserialized. 2. Filesystem Enumeration Ordering (ext4 vs NTFS): When scanning headers to build PCM module caches, directory enumeration (`readdir`) order differs between Linux (ext4 hash order) and Windows (NTFS alphabetical order). This difference is understood to affect PCM submodule layout and lazy AST deserialization chains. On Linux, static initialization chains in imported submodules appear to deserialize and emit `@<!-- -->hypotf` prior to the builtin call; CodeGen subsequently reused the existing symbol (with `dllimport`). On Windows, the ordering deferred this deserialization, leaving IR empty when `getBuiltinLibFunction` was called. This patch guarantees deterministic behavior across platforms by explicitly looking up the real library function declaration in the AST translation unit during `getBuiltinLibFunction` and using its `GlobalDecl` so attributes are always correctly inherited. When matching lookup candidates, we use `hasSameFunctionTypeIgnoringExceptionSpec` rather than exact type equality. Compiler builtins are implicitly declared `noexcept` (e.g. `float (float, float) noexcept`), whereas standard C library headers often declare the function without `noexcept`. In C++17 and later, exception specifications are part of `FunctionType`. Ignoring exception specifications ensures we correctly match the system header declaration without dropping proper compiler builtin `NoThrow` semantics. Adds a regression test verifying that calling `__builtin_hypotf` when `hypotf` is declared with `dllimport` in a module correctly produces `declare dllimport float @<!-- -->hypotf`. This is to fix error like https://ci.chromium.org/ui/p/chromium/builders/ci/Win%20x64%20Builder%20%28dbg%29/190643/overview happened in chromium's CI. Internal bug: http://b/525295309 Assisted-by: Gemini --- Full diff: https://github.com/llvm/llvm-project/pull/204792.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGBuiltin.cpp (+18) - (added) clang/test/Modules/builtin-hypotf-dllimport.cpp (+26) ``````````diff diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 509ab4245d99a..031381e592130 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -247,6 +247,24 @@ llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD, Name = Context.BuiltinInfo.getName(BuiltinID).substr(10); } + // If the library function has already been declared in the AST (e.g. from an + // imported header or module), look it up and use its GlobalDecl so that + // attributes (such as dllimport) are correctly attached. Builtins are often + // declared noexcept while standard C library headers may lack noexcept, so + // we ignore exception specifications when comparing function types. + if (const DeclContext *TU = Context.getTranslationUnitDecl()) { + IdentifierInfo &II = Context.Idents.get(Name); + for (const auto *Result : TU->lookup(&II)) { + if (const auto *LibFD = dyn_cast<FunctionDecl>(Result)) { + if (Context.hasSameFunctionTypeIgnoringExceptionSpec(LibFD->getType(), + FD->getType())) { + D = GlobalDecl(LibFD); + break; + } + } + } + } + llvm::FunctionType *Ty = cast<llvm::FunctionType>(getTypes().ConvertType(FD->getType())); diff --git a/clang/test/Modules/builtin-hypotf-dllimport.cpp b/clang/test/Modules/builtin-hypotf-dllimport.cpp new file mode 100644 index 0000000000000..5142f484b0498 --- /dev/null +++ b/clang/test/Modules/builtin-hypotf-dllimport.cpp @@ -0,0 +1,26 @@ +// Test that calling a builtin library function (e.g. __builtin_hypotf) correctly +// looks up the real standard library declaration from an imported module or header +// and attaches attributes such as dllimport, even if the builtin is implicitly +// declared noexcept while the C standard library declaration lacks noexcept. + +// RUN: rm -rf %t && mkdir %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fdeclspec -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -I%t -emit-llvm %t/main.cpp -o - | FileCheck %s + +// CHECK: declare dllimport float @hypotf(float noundef, float noundef) + +//--- math.h +extern "C" __declspec(dllimport) float hypotf(float x, float y); + +//--- module.modulemap +module Math { + header "math.h" +} + +//--- main.cpp +#include "math.h" + +extern "C" float test_func(float x, float y) { + return __builtin_hypotf(x, y); +} `````````` </details> https://github.com/llvm/llvm-project/pull/204792 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
