[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.

[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?

[efriedma-quic]

I don't know of any usage this limit would break, but gcc doesn't impose an equivalent limit, so there could be some code out there doing something weird.

Also, 32 is probably not a good limit for targets with 16-bit pointers, although this sort of construct should be rare on such targets.

[AaronBallman]

Can we keep a test where the result is narrowed to show that we reject those, or is that already covered elsewhere?

[tbaederr]

Yes, in clang/test/CodeGen/const-label-addr.c for example:

int b(void) {
  static int ar = &&l2 - &&l1;
l1:
  return 10;
l2:
  return 11;
}

[AaronBallman]

That's not what I was after, I meant a Sema test showing we diagnose:

int b(void) {
  static unsigned char ar = &&l2 - &&l1; // error
l1:
  return 10;
l2:
  return 11;
}

[tbaederr]

Oh, I guess we don't?
We also don't reject widening casts in sema today:

int b(void) {
  static unsigned __int128 ar = &&l2 - &&l1; // error
l1:
  return 10;
l2:
  return 11;
}

but we don't constant fold them.

[AaronBallman]

I was basing this off the patch description saying: Reject integral casts of addr-label-diffs if the result is narrower than 32 bits. ;-)

[tbaederr]

This patch rejects those casts in the same way we've always rejected widening casts :P