[clang] [clang] Fix crashes when passing VLA to va_arg (PR #119563)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 13 21:43:35 PST 2024
https://github.com/amane-ame updated https://github.com/llvm/llvm-project/pull/119563
>From 659eda3ec76b63418f8b621b004728d9d7bf26ad Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Wed, 11 Dec 2024 22:17:51 +0800
Subject: [PATCH 1/7] [clang] Fix crashes when passing VLA to va_arg
---
clang/lib/CodeGen/CGExprAgg.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 2ad6587089f101..a4111cb65c8b1c 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -2201,6 +2201,8 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
// But note that getTypeInfo returns 0 for a VLA.
if (auto *VAT = dyn_cast_or_null<VariableArrayType>(
getContext().getAsArrayType(Ty))) {
+ assert(Ty->isVariableArrayType());
+ EmitVariablyModifiedType(Ty);
QualType BaseEltTy;
SizeVal = emitArrayLength(VAT, BaseEltTy, DestPtr);
TypeInfo = getContext().getTypeInfoInChars(BaseEltTy);
>From 5937db790ff0a59ea5bf18cb008d38a4524dc7dc Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Thu, 12 Dec 2024 13:50:13 +0800
Subject: [PATCH 2/7] [clang] Add a testcase for passing VLA to va_arg
---
clang/test/CodeGen/varargs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/test/CodeGen/varargs.c b/clang/test/CodeGen/varargs.c
index 625399b87f7ad7..b7b1b52156be37 100644
--- a/clang/test/CodeGen/varargs.c
+++ b/clang/test/CodeGen/varargs.c
@@ -20,4 +20,7 @@ void vla(int n, ...)
__builtin_va_list ap;
void *p;
p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 {{.*}}, 1
+ // Don't crash on some undefined behaviors.
+ p = __builtin_va_arg(ap, typeof (int [++n]));
+ p = __builtin_va_arg(ap, typeof (int [n][n]));
}
>From df9f8f61ee21b81c9cfd300d113afea9298b8067 Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Thu, 12 Dec 2024 13:52:59 +0800
Subject: [PATCH 3/7] [clang] Move the parsing of VLA in va_arg to EmitVAArg
---
clang/lib/CodeGen/CGCall.cpp | 2 ++
clang/lib/CodeGen/CGExprAgg.cpp | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 3cefc9da66ddb8..4e2812c62f4357 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -6121,6 +6121,8 @@ RValue CodeGenFunction::EmitVAArg(VAArgExpr *VE, Address &VAListAddr,
VAListAddr = VE->isMicrosoftABI() ? EmitMSVAListRef(VE->getSubExpr())
: EmitVAListRef(VE->getSubExpr());
QualType Ty = VE->getType();
+ if (Ty->isVariableArrayType())
+ EmitVariablyModifiedType(Ty);
if (VE->isMicrosoftABI())
return CGM.getABIInfo().EmitMSVAArg(*this, VAListAddr, Ty, Slot);
return CGM.getABIInfo().EmitVAArg(*this, VAListAddr, Ty, Slot);
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index a4111cb65c8b1c..2ad6587089f101 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -2201,8 +2201,6 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
// But note that getTypeInfo returns 0 for a VLA.
if (auto *VAT = dyn_cast_or_null<VariableArrayType>(
getContext().getAsArrayType(Ty))) {
- assert(Ty->isVariableArrayType());
- EmitVariablyModifiedType(Ty);
QualType BaseEltTy;
SizeVal = emitArrayLength(VAT, BaseEltTy, DestPtr);
TypeInfo = getContext().getTypeInfoInChars(BaseEltTy);
>From b38c1d1ee20d3308a4120c3b95a167a936314a6b Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Thu, 12 Dec 2024 15:43:35 +0800
Subject: [PATCH 4/7] [clang] Emit an undefined-behavior warning for passing
VLA to va_arg
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++
clang/lib/Sema/SemaExpr.cpp | 7 +++++++
clang/test/CodeGen/varargs.c | 6 +++---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0a245e2077f68f..3a352f23faa353 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10497,6 +10497,10 @@ def warn_second_parameter_to_va_arg_ownership_qualified : Warning<
def warn_second_parameter_to_va_arg_never_compatible : Warning<
"second argument to 'va_arg' is of promotable type %0; this va_arg has "
"undefined behavior because arguments will be promoted to %1">, InGroup<Varargs>;
+def warn_second_parameter_to_va_arg_vla : Warning<
+ "second argument to 'va_arg' is of variable length array type %0; "
+ "this va_arg has undefined behavior because arguments will never "
+ "be compatible with variable length array type">, InGroup<Varargs>;
def warn_return_missing_expr : Warning<
"non-void %select{function|method}1 %0 should return a value">, DefaultError,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1524622408a18a..ad84f38b75cfcf 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16538,6 +16538,13 @@ ExprResult Sema::BuildVAArgExpr(SourceLocation BuiltinLoc,
<< TInfo->getTypeLoc().getSourceRange();
}
+ if (TInfo->getType()->isVariableArrayType()) {
+ DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
+ PDiag(diag::warn_second_parameter_to_va_arg_vla)
+ << TInfo->getType()
+ << TInfo->getTypeLoc().getSourceRange());
+ }
+
// Check for va_arg where arguments of the given type will be promoted
// (i.e. this va_arg is guaranteed to have undefined behavior).
QualType PromoteType;
diff --git a/clang/test/CodeGen/varargs.c b/clang/test/CodeGen/varargs.c
index b7b1b52156be37..8e474ef35b12f3 100644
--- a/clang/test/CodeGen/varargs.c
+++ b/clang/test/CodeGen/varargs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -verify -o - %s | FileCheck %s
// PR6433 - Don't crash on va_arg(typedef).
typedef double gdouble;
@@ -21,6 +21,6 @@ void vla(int n, ...)
void *p;
p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 {{.*}}, 1
// Don't crash on some undefined behaviors.
- p = __builtin_va_arg(ap, typeof (int [++n]));
- p = __builtin_va_arg(ap, typeof (int [n][n]));
+ p = __builtin_va_arg(ap, typeof (int [++n])); // expected-warning{{second argument to 'va_arg' is of variable length array type 'typeof(int[++n])'}}
+ p = __builtin_va_arg(ap, typeof (int [n][n])); // expected-warning{{second argument to 'va_arg' is of variable length array type 'typeof(int[n][n])'}}
}
>From 489e6b4a542b31026d242f48553ff3051a86c97e Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Sat, 14 Dec 2024 13:31:35 +0800
Subject: [PATCH 5/7] [clang] Move warning checks of va_arg from test/CodeGen
to test/Sema
---
clang/test/CodeGen/varargs.c | 5 +----
clang/test/Sema/varargs.c | 5 +++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/test/CodeGen/varargs.c b/clang/test/CodeGen/varargs.c
index 8e474ef35b12f3..625399b87f7ad7 100644
--- a/clang/test/CodeGen/varargs.c
+++ b/clang/test/CodeGen/varargs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -verify -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s
// PR6433 - Don't crash on va_arg(typedef).
typedef double gdouble;
@@ -20,7 +20,4 @@ void vla(int n, ...)
__builtin_va_list ap;
void *p;
p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 {{.*}}, 1
- // Don't crash on some undefined behaviors.
- p = __builtin_va_arg(ap, typeof (int [++n])); // expected-warning{{second argument to 'va_arg' is of variable length array type 'typeof(int[++n])'}}
- p = __builtin_va_arg(ap, typeof (int [n][n])); // expected-warning{{second argument to 'va_arg' is of variable length array type 'typeof(int[n][n])'}}
}
diff --git a/clang/test/Sema/varargs.c b/clang/test/Sema/varargs.c
index 2cb7270f604a0f..bec41dda65d571 100644
--- a/clang/test/Sema/varargs.c
+++ b/clang/test/Sema/varargs.c
@@ -75,6 +75,11 @@ void f9(__builtin_va_list args)
(void)__builtin_va_arg(args, enum E); // Don't warn here in C
(void)__builtin_va_arg(args, short); // expected-warning {{second argument to 'va_arg' is of promotable type 'short'}}
(void)__builtin_va_arg(args, char); // expected-warning {{second argument to 'va_arg' is of promotable type 'char'}}
+ // Don't crash on some undefined behaviors.
+ int n;
+ (void)__builtin_va_arg(args, int[10]); // expected-warning{{second argument to 'va_arg' is of array type 'int[10]'}}
+ (void)__builtin_va_arg(args, int[++n]); // expected-warning{{second argument to 'va_arg' is of array type 'int[++n]'}}
+ (void)__builtin_va_arg(args, int[n][n]); // expected-warning{{second argument to 'va_arg' is of array type 'int[n][n]'}}
}
void f10(int a, ...) {
>From 0888e156abc0631267f9f518378f78b1c856a8d7 Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Sat, 14 Dec 2024 13:33:16 +0800
Subject: [PATCH 6/7] [clang] Emit a warning for passing any array type to
va_arg
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 8 ++++----
clang/lib/Sema/SemaExpr.cpp | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3a352f23faa353..eb466a45cc05a7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10497,10 +10497,10 @@ def warn_second_parameter_to_va_arg_ownership_qualified : Warning<
def warn_second_parameter_to_va_arg_never_compatible : Warning<
"second argument to 'va_arg' is of promotable type %0; this va_arg has "
"undefined behavior because arguments will be promoted to %1">, InGroup<Varargs>;
-def warn_second_parameter_to_va_arg_vla : Warning<
- "second argument to 'va_arg' is of variable length array type %0; "
- "this va_arg has undefined behavior because arguments will never "
- "be compatible with variable length array type">, InGroup<Varargs>;
+def warn_second_parameter_to_va_arg_array : Warning<
+ "second argument to 'va_arg' is of array type %0; "
+ "this va_arg has undefined behavior because arguments "
+ "will never be compatible with array type">, InGroup<Varargs>;
def warn_return_missing_expr : Warning<
"non-void %select{function|method}1 %0 should return a value">, DefaultError,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ad84f38b75cfcf..fca678e024caf2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16538,9 +16538,9 @@ ExprResult Sema::BuildVAArgExpr(SourceLocation BuiltinLoc,
<< TInfo->getTypeLoc().getSourceRange();
}
- if (TInfo->getType()->isVariableArrayType()) {
+ if (TInfo->getType()->isArrayType()) {
DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
- PDiag(diag::warn_second_parameter_to_va_arg_vla)
+ PDiag(diag::warn_second_parameter_to_va_arg_array)
<< TInfo->getType()
<< TInfo->getTypeLoc().getSourceRange());
}
>From d94f84a7c7574a8eb33eecb9420b573c799e07ca Mon Sep 17 00:00:00 2001
From: amane-ame <i at amane-a.me>
Date: Sat, 14 Dec 2024 13:43:10 +0800
Subject: [PATCH 7/7] [clang] Drop the call to EmitVariablyModifiedType in
ScalarExprEmitter::VisitVAArgExpr
---
clang/lib/CodeGen/CGCall.cpp | 2 +-
clang/lib/CodeGen/CGExprScalar.cpp | 5 -----
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 4e2812c62f4357..798354f2c8c241 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -6121,7 +6121,7 @@ RValue CodeGenFunction::EmitVAArg(VAArgExpr *VE, Address &VAListAddr,
VAListAddr = VE->isMicrosoftABI() ? EmitMSVAListRef(VE->getSubExpr())
: EmitVAListRef(VE->getSubExpr());
QualType Ty = VE->getType();
- if (Ty->isVariableArrayType())
+ if (Ty->isVariablyModifiedType())
EmitVariablyModifiedType(Ty);
if (VE->isMicrosoftABI())
return CGM.getABIInfo().EmitMSVAArg(*this, VAListAddr, Ty, Slot);
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index bbf68a4c66192a..0ea357b43b5b5a 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -5448,11 +5448,6 @@ Value *ScalarExprEmitter::VisitChooseExpr(ChooseExpr *E) {
}
Value *ScalarExprEmitter::VisitVAArgExpr(VAArgExpr *VE) {
- QualType Ty = VE->getType();
-
- if (Ty->isVariablyModifiedType())
- CGF.EmitVariablyModifiedType(Ty);
-
Address ArgValue = Address::invalid();
RValue ArgPtr = CGF.EmitVAArg(VE, ArgValue);
More information about the cfe-commits
mailing list