From 0f9aade16e1591f34b04ed81ab0e0ec42340c312 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 12 May 2025 19:04:19 -0400 Subject: [PATCH 1/3] [Clang] Fix Sema::checkArgCount when calling a function that wants zero arguments with one argument In this case, `Call->getArg(1)` will trap when trying to format the diagnostic. It also improves the rendering of the diagnostic some of the time: Before: ``` $ ./bin/clang -c a.c a.c:2:30: error: too many arguments to function call, expected 2, have 4 2 | __builtin_annotation(1, 2, 3, 4); | ~ ^ ``` After: ``` $ ./bin/clang -c a.c a.c:2:30: error: too many arguments to function call, expected 2, have 4 2 | __builtin_annotation(1, 2, 3, 4); | ^~~~ ``` Split from #139580 --- clang/lib/Sema/SemaChecking.cpp | 2 +- clang/lib/Sema/SemaWasm.cpp | 8 ++------ clang/test/Sema/builtins-wasm.c | 1 + clang/test/Sema/builtins.c | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 97f623f61a405..5d0d5861d4026 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -168,7 +168,7 @@ bool Sema::checkArgCount(CallExpr *Call, unsigned DesiredArgCount) { return Diag(Range.getBegin(), diag::err_typecheck_call_too_many_args) << 0 /*function call*/ << DesiredArgCount << ArgCount - << /*is non object*/ 0 << Call->getArg(1)->getSourceRange(); + << /*is non object*/ 0 << Range; } static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { diff --git a/clang/lib/Sema/SemaWasm.cpp b/clang/lib/Sema/SemaWasm.cpp index c0fa05bc17609..a76601f38e404 100644 --- a/clang/lib/Sema/SemaWasm.cpp +++ b/clang/lib/Sema/SemaWasm.cpp @@ -52,7 +52,7 @@ static bool CheckWasmBuiltinArgIsInteger(Sema &S, CallExpr *E, } bool SemaWasm::BuiltinWasmRefNullExtern(CallExpr *TheCall) { - if (TheCall->getNumArgs() != 0) + if (SemaRef.checkArgCount(TheCall, 0)) return true; TheCall->setType(getASTContext().getWebAssemblyExternrefType()); @@ -62,12 +62,8 @@ bool SemaWasm::BuiltinWasmRefNullExtern(CallExpr *TheCall) { bool SemaWasm::BuiltinWasmRefNullFunc(CallExpr *TheCall) { ASTContext &Context = getASTContext(); - if (TheCall->getNumArgs() != 0) { - Diag(TheCall->getBeginLoc(), diag::err_typecheck_call_too_many_args) - << 0 /*function call*/ << /*expected*/ 0 << TheCall->getNumArgs() - << /*is non object*/ 0; + if (SemaRef.checkArgCount(TheCall, 0)) return true; - } // This custom type checking code ensures that the nodes are as expected // in order to later on generate the necessary builtin. diff --git a/clang/test/Sema/builtins-wasm.c b/clang/test/Sema/builtins-wasm.c index beb430616233a..1aae365c95aff 100644 --- a/clang/test/Sema/builtins-wasm.c +++ b/clang/test/Sema/builtins-wasm.c @@ -7,6 +7,7 @@ static __externref_t table[0]; typedef void (*__funcref funcref_t)(); void test_ref_null() { funcref_t func = __builtin_wasm_ref_null_func(0); // expected-error {{too many arguments to function call, expected 0, have 1}} + __externref_t ref = __builtin_wasm_ref_null_extern(0); // expected-error {{too many arguments to function call, expected 0, have 1}} } void test_table_size(__externref_t ref, void *ptr, int arr[]) { diff --git a/clang/test/Sema/builtins.c b/clang/test/Sema/builtins.c index b669ee68cdd95..310079c8f4ed0 100644 --- a/clang/test/Sema/builtins.c +++ b/clang/test/Sema/builtins.c @@ -75,7 +75,7 @@ void test10(void) __attribute__((noreturn)); void test10(void) { __asm__("int3"); - __builtin_unreachable(); + __builtin_unreachable(1, 2, 3, 6, 7); // No warning about falling off the end of a noreturn function. } From f0ca59b0475a12f3e0f9b080f34aee3236994b8e Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 12 May 2025 19:21:33 -0400 Subject: [PATCH 2/3] Revert accidental change --- clang/test/Sema/builtins.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Sema/builtins.c b/clang/test/Sema/builtins.c index 310079c8f4ed0..b669ee68cdd95 100644 --- a/clang/test/Sema/builtins.c +++ b/clang/test/Sema/builtins.c @@ -75,7 +75,7 @@ void test10(void) __attribute__((noreturn)); void test10(void) { __asm__("int3"); - __builtin_unreachable(1, 2, 3, 6, 7); + __builtin_unreachable(); // No warning about falling off the end of a noreturn function. } From 94b74974a1a3e52ec216f684a7308fb34cf1813e Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 13 May 2025 11:22:31 -0400 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Mariya Podchishchaeva --- clang/lib/Sema/SemaWasm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaWasm.cpp b/clang/lib/Sema/SemaWasm.cpp index a76601f38e404..3362e1d717a6c 100644 --- a/clang/lib/Sema/SemaWasm.cpp +++ b/clang/lib/Sema/SemaWasm.cpp @@ -52,7 +52,7 @@ static bool CheckWasmBuiltinArgIsInteger(Sema &S, CallExpr *E, } bool SemaWasm::BuiltinWasmRefNullExtern(CallExpr *TheCall) { - if (SemaRef.checkArgCount(TheCall, 0)) + if (SemaRef.checkArgCount(TheCall, /*DesiredArgCount=*/0)) return true; TheCall->setType(getASTContext().getWebAssemblyExternrefType()); @@ -62,7 +62,7 @@ bool SemaWasm::BuiltinWasmRefNullExtern(CallExpr *TheCall) { bool SemaWasm::BuiltinWasmRefNullFunc(CallExpr *TheCall) { ASTContext &Context = getASTContext(); - if (SemaRef.checkArgCount(TheCall, 0)) + if (SemaRef.checkArgCount(TheCall, /*DesiredArgCount=*/0)) return true; // This custom type checking code ensures that the nodes are as expected