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);