Commit ec2c299c authored by Tobias Tebbi's avatar Tobias Tebbi Committed by Commit Bot

[torque] check runtime function signatures properly

The missing check that runtime function parameters have to be tagged
lead to a bug: https://chromium-review.googlesource.com/c/v8/v8/+/1604071

drive-by-fix: Remove obsolete verbose mode. It hasn't been maintained
since the very early Torque versions, and the remaining printf's are
rather useless.

Bug: v8:7793
Change-Id: I59adf4c6c5d92a8838cdc638afb2ab7a41550b55
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1609910
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61483}
parent 4329354a
...@@ -132,27 +132,28 @@ Builtin* DeclarationVisitor::CreateBuiltin(BuiltinDeclaration* decl, ...@@ -132,27 +132,28 @@ Builtin* DeclarationVisitor::CreateBuiltin(BuiltinDeclaration* decl,
void DeclarationVisitor::Visit(ExternalRuntimeDeclaration* decl, void DeclarationVisitor::Visit(ExternalRuntimeDeclaration* decl,
const Signature& signature, const Signature& signature,
base::Optional<Statement*> body) { base::Optional<Statement*> body) {
if (GlobalContext::verbose()) {
std::cout << "found declaration of external runtime " << decl->name
<< " with signature ";
}
if (signature.parameter_types.types.size() == 0 || if (signature.parameter_types.types.size() == 0 ||
!(signature.parameter_types.types[0] == !(signature.parameter_types.types[0] ==
Declarations::LookupGlobalType(CONTEXT_TYPE_STRING))) { Declarations::LookupGlobalType(CONTEXT_TYPE_STRING))) {
std::stringstream stream; ReportError(
stream << "first parameter to runtime " << decl->name "first parameter to runtime functions has to be the context and have "
<< " is not a context but should be"; "type Context, but found type ",
ReportError(stream.str()); signature.parameter_types.types[0]);
} }
if (!(signature.return_type->IsSubtypeOf(TypeOracle::GetObjectType()) ||
if (signature.return_type->IsStructType()) { signature.return_type == TypeOracle::GetVoidType() ||
std::stringstream stream; signature.return_type == TypeOracle::GetNeverType())) {
stream << "runtime functions (in this case" << decl->name ReportError(
<< ") cannot return structs (in this case " "runtime functions can only return tagged values, but found type ",
<< static_cast<const StructType*>(signature.return_type)->name() signature.return_type);
<< ")"; }
ReportError(stream.str()); for (const Type* parameter_type : signature.parameter_types.types) {
if (!parameter_type->IsSubtypeOf(TypeOracle::GetObjectType())) {
ReportError(
"runtime functions can only take tagged values as parameters, but "
"found type ",
*parameter_type);
}
} }
Declarations::DeclareRuntimeFunction(decl->name, signature, Declarations::DeclareRuntimeFunction(decl->name, signature,
...@@ -162,11 +163,6 @@ void DeclarationVisitor::Visit(ExternalRuntimeDeclaration* decl, ...@@ -162,11 +163,6 @@ void DeclarationVisitor::Visit(ExternalRuntimeDeclaration* decl,
void DeclarationVisitor::Visit(ExternalMacroDeclaration* decl, void DeclarationVisitor::Visit(ExternalMacroDeclaration* decl,
const Signature& signature, const Signature& signature,
base::Optional<Statement*> body) { base::Optional<Statement*> body) {
if (GlobalContext::verbose()) {
std::cout << "found declaration of external macro " << decl->name
<< " with signature ";
}
Declarations::DeclareMacro(decl->name, decl->external_assembler_name, Declarations::DeclareMacro(decl->name, decl->external_assembler_name,
signature, decl->transitioning, body, decl->op); signature, decl->transitioning, body, decl->op);
} }
......
...@@ -20,8 +20,7 @@ class GlobalContext : public ContextualClass<GlobalContext> { ...@@ -20,8 +20,7 @@ class GlobalContext : public ContextualClass<GlobalContext> {
GlobalContext(GlobalContext&&) V8_NOEXCEPT = default; GlobalContext(GlobalContext&&) V8_NOEXCEPT = default;
GlobalContext& operator=(GlobalContext&&) V8_NOEXCEPT = default; GlobalContext& operator=(GlobalContext&&) V8_NOEXCEPT = default;
explicit GlobalContext(Ast ast) explicit GlobalContext(Ast ast)
: verbose_(false), : collect_language_server_data_(false),
collect_language_server_data_(false),
force_assert_statements_(false), force_assert_statements_(false),
ast_(std::move(ast)) { ast_(std::move(ast)) {
CurrentScope::Scope current_scope(nullptr); CurrentScope::Scope current_scope(nullptr);
...@@ -67,8 +66,6 @@ class GlobalContext : public ContextualClass<GlobalContext> { ...@@ -67,8 +66,6 @@ class GlobalContext : public ContextualClass<GlobalContext> {
return Get().cpp_includes_; return Get().cpp_includes_;
} }
static void SetVerbose() { Get().verbose_ = true; }
static bool verbose() { return Get().verbose_; }
static void SetCollectLanguageServerData() { static void SetCollectLanguageServerData() {
Get().collect_language_server_data_ = true; Get().collect_language_server_data_ = true;
} }
...@@ -84,7 +81,6 @@ class GlobalContext : public ContextualClass<GlobalContext> { ...@@ -84,7 +81,6 @@ class GlobalContext : public ContextualClass<GlobalContext> {
static Ast* ast() { return &Get().ast_; } static Ast* ast() { return &Get().ast_; }
private: private:
bool verbose_;
bool collect_language_server_data_; bool collect_language_server_data_;
bool force_assert_statements_; bool force_assert_statements_;
Namespace* default_namespace_; Namespace* default_namespace_;
......
...@@ -1597,10 +1597,6 @@ void ImplementationVisitor::GenerateMacroFunctionDeclaration( ...@@ -1597,10 +1597,6 @@ void ImplementationVisitor::GenerateMacroFunctionDeclaration(
void ImplementationVisitor::GenerateFunctionDeclaration( void ImplementationVisitor::GenerateFunctionDeclaration(
std::ostream& o, const std::string& macro_prefix, const std::string& name, std::ostream& o, const std::string& macro_prefix, const std::string& name,
const Signature& signature, const NameVector& parameter_names) { const Signature& signature, const NameVector& parameter_names) {
if (GlobalContext::verbose()) {
std::cout << "generating source for declaration " << name << "\n";
}
if (signature.return_type->IsVoidOrNever()) { if (signature.return_type->IsVoidOrNever()) {
o << "void"; o << "void";
} else { } else {
...@@ -2235,11 +2231,6 @@ VisitResult ImplementationVisitor::GenerateCall( ...@@ -2235,11 +2231,6 @@ VisitResult ImplementationVisitor::GenerateCall(
&argument_range, &constexpr_arguments); &argument_range, &constexpr_arguments);
} }
if (GlobalContext::verbose()) {
std::cout << "generating code for call to " << callable->ReadableName()
<< "\n";
}
size_t label_count = callable->signature().labels.size(); size_t label_count = callable->signature().labels.size();
if (label_count != arguments.labels.size()) { if (label_count != arguments.labels.size()) {
std::stringstream s; std::stringstream s;
......
...@@ -178,7 +178,6 @@ void RecompileTorque(MessageWriter writer) { ...@@ -178,7 +178,6 @@ void RecompileTorque(MessageWriter writer) {
TorqueCompilerOptions options; TorqueCompilerOptions options;
options.output_directory = ""; options.output_directory = "";
options.verbose = false;
options.collect_language_server_data = true; options.collect_language_server_data = true;
options.force_assert_statements = true; options.force_assert_statements = true;
......
...@@ -47,7 +47,6 @@ void ReadAndParseTorqueFile(const std::string& path) { ...@@ -47,7 +47,6 @@ void ReadAndParseTorqueFile(const std::string& path) {
void CompileCurrentAst(TorqueCompilerOptions options) { void CompileCurrentAst(TorqueCompilerOptions options) {
GlobalContext::Scope global_context(std::move(CurrentAst::Get())); GlobalContext::Scope global_context(std::move(CurrentAst::Get()));
if (options.verbose) GlobalContext::SetVerbose();
if (options.collect_language_server_data) { if (options.collect_language_server_data) {
GlobalContext::SetCollectLanguageServerData(); GlobalContext::SetCollectLanguageServerData();
} }
......
...@@ -17,7 +17,6 @@ namespace torque { ...@@ -17,7 +17,6 @@ namespace torque {
struct TorqueCompilerOptions { struct TorqueCompilerOptions {
std::string output_directory = ""; std::string output_directory = "";
bool verbose = false;
bool collect_language_server_data = false; bool collect_language_server_data = false;
// assert(...) are only generated for debug builds. The provide // assert(...) are only generated for debug builds. The provide
......
...@@ -11,7 +11,6 @@ namespace torque { ...@@ -11,7 +11,6 @@ namespace torque {
int WrappedMain(int argc, const char** argv) { int WrappedMain(int argc, const char** argv) {
std::string output_directory; std::string output_directory;
bool verbose = false;
std::vector<std::string> files; std::vector<std::string> files;
for (int i = 1; i < argc; ++i) { for (int i = 1; i < argc; ++i) {
...@@ -20,10 +19,6 @@ int WrappedMain(int argc, const char** argv) { ...@@ -20,10 +19,6 @@ int WrappedMain(int argc, const char** argv) {
output_directory = argv[++i]; output_directory = argv[++i];
continue; continue;
} }
if (!strcmp("-v", argv[i])) {
verbose = true;
continue;
}
// Otherwise it's a .tq file. Remember it for compilation. // Otherwise it's a .tq file. Remember it for compilation.
files.emplace_back(argv[i]); files.emplace_back(argv[i]);
...@@ -31,7 +26,6 @@ int WrappedMain(int argc, const char** argv) { ...@@ -31,7 +26,6 @@ int WrappedMain(int argc, const char** argv) {
TorqueCompilerOptions options; TorqueCompilerOptions options;
options.output_directory = output_directory; options.output_directory = output_directory;
options.verbose = verbose;
options.collect_language_server_data = false; options.collect_language_server_data = false;
options.force_assert_statements = false; options.force_assert_statements = false;
......
...@@ -19,7 +19,6 @@ struct TestCompiler { ...@@ -19,7 +19,6 @@ struct TestCompiler {
void Compile(const std::string& source) { void Compile(const std::string& source) {
TorqueCompilerOptions options; TorqueCompilerOptions options;
options.output_directory = ""; options.output_directory = "";
options.verbose = false;
options.collect_language_server_data = true; options.collect_language_server_data = true;
options.force_assert_statements = true; options.force_assert_statements = true;
......
...@@ -16,7 +16,6 @@ namespace { ...@@ -16,7 +16,6 @@ namespace {
TorqueCompilerResult TestCompileTorque(const std::string& source) { TorqueCompilerResult TestCompileTorque(const std::string& source) {
TorqueCompilerOptions options; TorqueCompilerOptions options;
options.output_directory = ""; options.output_directory = "";
options.verbose = false;
options.collect_language_server_data = false; options.collect_language_server_data = false;
options.force_assert_statements = false; options.force_assert_statements = false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment