Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tbaederr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment on lines +18614 to +18617
Copy link
Collaborator

@zygoloid zygoloid Dec 10, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

@efriedma-quic efriedma-quic Dec 10, 2025

Choose a reason for hiding this comment

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

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 .

Copy link
Collaborator

@zygoloid zygoloid Dec 10, 2025

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@efriedma-quic efriedma-quic Dec 10, 2025

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@zygoloid zygoloid Dec 10, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@zygoloid zygoloid Dec 10, 2025

Choose a reason for hiding this comment

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

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.

// Only allow casts of lvalues if they are lossless.
return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-APValue-addrlabeldiff.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

// CHECK: | |-value: AddrLabelDiff &&l2 - &&l1
int Test(void) {
constexpr char ar = &&l2 - &&l1;
constexpr long long ar = &&l2 - &&l1;
l1:
return 10;
l2:
Expand Down
4 changes: 4 additions & 0 deletions clang/test/CodeGenCXX/const-init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;}
Expand Down