From 8e98038133ea1913d13bb60146d5ffb49d9ef3a7 Mon Sep 17 00:00:00 2001 From: rootkiller6788 Date: Fri, 21 Aug 2026 20:19:23 +0800 Subject: [PATCH] Fix generator output for functions with a parameter named `ret` The return value slot of a generated wrapper function was named `v_ret_`, which collides with the local copy of a parameter named `ret` (parameter locals are built as `v__`, so `ret` becomes `v_ret_`). The generated code then declared `v_ret_` twice in the same scope, e.g. for libxslt functions that use `ret` as a parameter name. Rename the return value variable to `v_ret`, which cannot collide with any parameter-derived variable since those always carry a trailing underscore. Fixes #142 --- .../tools/clang_generator/emitter.cc | 10 +++++---- .../tools/clang_generator/emitter_test.cc | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/sandboxed_api/tools/clang_generator/emitter.cc b/sandboxed_api/tools/clang_generator/emitter.cc index f4663d45..68989cc2 100644 --- a/sandboxed_api/tools/clang_generator/emitter.cc +++ b/sandboxed_api/tools/clang_generator/emitter.cc @@ -447,8 +447,10 @@ absl::StatusOr Emitter::DoEmitFunction( absl::StrAppend(&out, ") {\n"); - // Declare the return value of the SAPI function. - absl::StrAppend(&out, type_mapper.MapQualType(return_type), " v_ret_;\n"); + // Declare the return value of the SAPI function. The variable is not + // suffixed with an underscore like the parameter variables below, so that a + // parameter named `ret` cannot collide with it. + absl::StrAppend(&out, type_mapper.MapQualType(return_type), " v_ret;\n"); // Declare the local variables for the parameters. for (const auto& [qual, name] : params) { @@ -460,7 +462,7 @@ absl::StatusOr Emitter::DoEmitFunction( // Call the sandboxed function. absl::StrAppend(&out, "\nABSL_RETURN_IF_ERROR(sandbox_->Call(\"", - function_name, "\", &v_ret_"); + function_name, "\", &v_ret"); for (const auto& [qual, name] : params) { absl::StrAppend(&out, ", ", IsPointerOrReference(qual) ? "" : "&v_", name); } @@ -468,7 +470,7 @@ absl::StatusOr Emitter::DoEmitFunction( // End the sandboxed function call and return `ok` if the unsandboxed function // returns void, or else return the value of the SAPI function. absl::StrAppend(&out, "));\nreturn ", - (returns_void ? "::absl::OkStatus()" : "v_ret_.GetValue()"), + (returns_void ? "::absl::OkStatus()" : "v_ret.GetValue()"), ";\n}\n"); return out; } diff --git a/sandboxed_api/tools/clang_generator/emitter_test.cc b/sandboxed_api/tools/clang_generator/emitter_test.cc index 93c2d089..f39f11b8 100644 --- a/sandboxed_api/tools/clang_generator/emitter_test.cc +++ b/sandboxed_api/tools/clang_generator/emitter_test.cc @@ -183,6 +183,27 @@ TEST_F(EmitterTest, AllFunctionsLimitScanDepthFailure) { EXPECT_THAT(emitter.GetRenderedFunctions(), IsEmpty()); } +// Tests that the generator produces valid code for functions that have a +// parameter named `ret`. The local copy of the parameter and the return value +// variable must not collide (both would be named `v_ret_`). +TEST_F(EmitterTest, ParameterNamedRet) { + GeneratorOptions options; + EmitterForTesting emitter(&options); + ASSERT_THAT(RunFrontendAction( + R"(extern "C" int FunctionWithRet(int ret);)", + std::make_unique(&emitter, &options)), + IsOk()); + EXPECT_THAT(emitter.GetRenderedFunctions(), SizeIs(1)); + + absl::StatusOr header = emitter.EmitHeader(); + ASSERT_THAT(header, IsOk()); + const std::string uglified = UglifyAll({*header})[0]; + + // The return value slot and the parameter copy must be passed to + // sandbox_->Call() as two distinct variables. + EXPECT_THAT(uglified, HasSubstr("&v_ret, &v_ret_")); +} + TEST_F(EmitterTest, RelatedTypes) { GeneratorOptions options; EmitterForTesting emitter(&options);