Skip to content

Conversation

@frederick-vs-ja
Copy link
Contributor

This patch makes Clang predefine _CRT_USE_BUILTIN_OFFSETOF in MS-compatible modes. The macro can make the offsetof provided by MS UCRT's <stddef.h> to select the __builtin_offsetof version, so with it Clang (Clang-cl) can directly consume UCRT's offsetof.

MSVC predefines the macro as 1 since at least VS 2017 19.14, but I think it's also OK to define it in "older" compatible modes.

Fixes #59689.

@frederick-vs-ja frederick-vs-ja added clang:frontend Language frontend issues, e.g. anything involving "Sema" platform:windows labels Feb 18, 2025
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 18, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-platform-windows

Author: A. Jiang (frederick-vs-ja)

Changes

This patch makes Clang predefine _CRT_USE_BUILTIN_OFFSETOF in MS-compatible modes. The macro can make the offsetof provided by MS UCRT's &lt;stddef.h&gt; to select the __builtin_offsetof version, so with it Clang (Clang-cl) can directly consume UCRT's offsetof.

MSVC predefines the macro as 1 since at least VS 2017 19.14, but I think it's also OK to define it in "older" compatible modes.

Fixes #59689.


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

3 Files Affected:

  • (modified) clang/lib/Basic/Targets/OSTargets.cpp (+2)
  • (added) clang/test/Sema/offsetof-ucrt.c (+16)
  • (added) clang/test/SemaCXX/offsetof-ucrt.cpp (+16)
diff --git a/clang/lib/Basic/Targets/OSTargets.cpp b/clang/lib/Basic/Targets/OSTargets.cpp
index 8af6623e5cb15..2e57c286c9a16 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -220,6 +220,8 @@ static void addVisualCDefines(const LangOptions &Opts, MacroBuilder &Builder) {
     Builder.defineMacro("_MSC_FULL_VER", Twine(Opts.MSCompatibilityVersion));
     // FIXME We cannot encode the revision information into 32-bits
     Builder.defineMacro("_MSC_BUILD", Twine(1));
+    // https://github.com/llvm/llvm-project/issues/59689
+    Builder.defineMacro("_CRT_USE_BUILTIN_OFFSETOF", Twine(1));
 
     if (Opts.CPlusPlus11 && Opts.isCompatibleWithMSVC(LangOptions::MSVC2015))
       Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
diff --git a/clang/test/Sema/offsetof-ucrt.c b/clang/test/Sema/offsetof-ucrt.c
new file mode 100644
index 0000000000000..0f21b317c1ff2
--- /dev/null
+++ b/clang/test/Sema/offsetof-ucrt.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -verify -fms-compatibility
+
+typedef __typeof__(sizeof(0)) size_t;
+
+#if defined _MSC_VER && !defined _CRT_USE_BUILTIN_OFFSETOF
+#ifdef __cplusplus
+#define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))
+#else
+#define offsetof(s,m) ((size_t)&(((s*)0)->m))
+#endif
+#else
+#define offsetof(s,m) __builtin_offsetof(s,m)
+#endif
+
+struct S { int a; };
+_Static_assert(offsetof(struct S, a) == 0, "");
diff --git a/clang/test/SemaCXX/offsetof-ucrt.cpp b/clang/test/SemaCXX/offsetof-ucrt.cpp
new file mode 100644
index 0000000000000..425e0e7f88f37
--- /dev/null
+++ b/clang/test/SemaCXX/offsetof-ucrt.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -verify -fms-compatibility
+
+typedef __typeof__(sizeof(0)) size_t;
+
+#if defined _MSC_VER && !defined _CRT_USE_BUILTIN_OFFSETOF
+#ifdef __cplusplus
+#define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))
+#else
+#define offsetof(s,m) ((size_t)&(((s*)0)->m))
+#endif
+#else
+#define offsetof(s,m) __builtin_offsetof(s,m)
+#endif
+
+struct S { int a; };
+_Static_assert(offsetof(S, a) == 0, "");

@frederick-vs-ja frederick-vs-ja force-pushed the __builtin_offsetof-with-ucrt branch from 913b74f to f10028a Compare February 18, 2025 05:57
This patch makes Clang predefine `_CRT_USE_BUILTIN_OFFSETOF` in
MS-compatible modes. The macro can make the offsetof provided by MS
UCRT's `<stddef.h>` to select the `__builtin_offsetof` version,
so with it Clang (Clang-cl) can directly consume UCRT's `offsetof`.

MSVC predefines the macro as `1` since at least VS 2017 19.14, but I
think it's also OK to define it in "older" compatible modes.
@frederick-vs-ja frederick-vs-ja force-pushed the __builtin_offsetof-with-ucrt branch from f10028a to 2c9e6e4 Compare February 18, 2025 06:03
@AaronBallman AaronBallman self-requested a review February 19, 2025 14:30
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! When landing, please add a release note to clang/docs/ReleaseNotes.rst so users know about the change.

@frederick-vs-ja frederick-vs-ja merged commit 6abe19a into llvm:main Mar 13, 2025
12 checks passed
@frederick-vs-ja frederick-vs-ja deleted the __builtin_offsetof-with-ucrt branch March 13, 2025 06:02
hach-que pushed a commit to RedpointGames/llvm-project that referenced this pull request Sep 5, 2025
…lvm#127568)

This patch makes Clang predefine `_CRT_USE_BUILTIN_OFFSETOF` in
MS-compatible modes. The macro can make the `offsetof` provided by MS
UCRT's `<stddef.h>` to select the `__builtin_offsetof` version, so with
it Clang (Clang-cl) can directly consume UCRT's `offsetof`.

MSVC predefines the macro as `1` since at least VS 2017 19.14, but I
think it's also OK to define it in "older" compatible modes.

Fixes llvm#59689.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category platform:windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

offsetof() is not constant in clang-cl

3 participants