[clang][ExprConstant] Reject integral casts of addr-label-diffs...

[tbaederr]

... if the result is narrower than 32 bits.

See the discussion in #136135

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

... if the result is narrower than 32 bits.

See the discussion in #136135

---

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

2 Files Affected:

• (modified) clang/lib/AST/ExprConstant.cpp (+6-3)
• (modified) clang/test/CodeGenCXX/const-init.cpp (+4)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index d81496ffd74e0..bcc896afac64e 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -18606,12 +18606,15 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
 
     if (!Result.isInt()) {
       // Allow casts of address-of-label differences if they are no-ops
-      // or narrowing.  (The narrowing case isn't actually guaranteed to
+      // or narrowing, if the result is at least 32 bits wide.
+      // (The narrowing case isn't actually guaranteed to
       // be constant-evaluatable except in some narrow cases which are hard
       // to detect here.  We let it through on the assumption the user knows
       // what they are doing.)
-      if (Result.isAddrLabelDiff())
-        return Info.Ctx.getTypeSize(DestType) <= Info.Ctx.getTypeSize(SrcType);
+      if (Result.isAddrLabelDiff()) {
+        unsigned DestBits = Info.Ctx.getTypeSize(DestType);
+        return DestBits >= 32 && DestBits <= Info.Ctx.getTypeSize(SrcType);
+      }
       // Only allow casts of lvalues if they are lossless.
       return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
     }
diff --git a/clang/test/CodeGenCXX/const-init.cpp b/clang/test/CodeGenCXX/const-init.cpp
index fd6fd24ae1d94..f5b715949f23a 100644
--- a/clang/test/CodeGenCXX/const-init.cpp
+++ b/clang/test/CodeGenCXX/const-init.cpp
@@ -73,6 +73,10 @@ __int128_t PR11705 = (__int128_t)&PR11705;
 // CHECK: @_ZZ23UnfoldableAddrLabelDiffvE1x = internal global i128 0
 void UnfoldableAddrLabelDiff() { static __int128_t x = (long)&&a-(long)&&b; a:b:return;}
 
+// CHECK: @_ZZ24UnfoldableAddrLabelDiff2vE1x = internal global i16 0
+void UnfoldableAddrLabelDiff2() { static short x = (long)&&a-(long)&&b; a:b:return;}
+
+
 // But make sure we do fold this.
 // CHECK: @_ZZ21FoldableAddrLabelDiffvE1x = internal global i64 sub (i64 ptrtoint (ptr blockaddress(@_Z21FoldableAddrLabelDiffv
 void FoldableAddrLabelDiff() { static long x = (long)&&a-(long)&&b; a:b:return;}

[github-actions]

🐧 Linux x64 Test Results

• 111848 tests passed
• 4492 tests skipped

✅ The build succeeded and all tests passed.

[github-actions]

🪟 Windows x64 Test Results

• 52922 tests passed
• 2057 tests skipped

✅ The build succeeded and all tests passed.

[efriedma-quic]

What's the benefit of hardcoding "32" here? Does it help simplify the interpreter implementation somehow?

If it does, this is fine, I guess.

[zygoloid]

Can we get away with dropping this special case entirely, and using the general rule from a couple of lines below that only lossless casts are allowed?

[efriedma-quic]

No, we actually need to allow casts that aren't lossless. See, for example, https://github.com/llvm/llvm-test-suite/blob/main/SingleSource/Regression/C/gcc-c-torture/execute/pr70460.c .

[zygoloid]

That's a shame. I assume we believe that was reduced from something real rather than just being a compiler torture test?

[efriedma-quic]

The bug report says "The following testcase distilled from glibc's vfprintf [...]".

[zygoloid]

Answering my own question: in the first couple of pages of github search results, I found a bunch of places initializing a static int[] [GHC] [EmbeddedCommonLisp [Juce] [CZ80] -- and in fact no places using any other type for a static array (no intptr_t, nothing smaller than int).

In light of that, the lower bound of 32 seems OK to me.

[zygoloid]

The bug report says "The following testcase distilled from glibc's vfprintf [...]".

Ah, the macro obscured this enough that my regex didn't find this one. That's still there, and has a comment saying they're intentionally using int even on 64-bit systems to save space.

[tbaederr]

What's the benefit of hardcoding "32" here? Does it help simplify the interpreter implementation somehow?

I hardcoded 32 because you said "I don't think there's any reason anyone would want to go narrower than 32 bits," in #136135, would you query the integer bitwidth or do something else?