From d3eae6d1f003af1bd5e4733e9abea8e4453d82a5 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Thu, 16 Nov 2023 10:26:39 -0800 Subject: [PATCH] Fix a crash on invalid found by fuzzing. (#3404) When a `namespace` keyword has no `;` following it, we recover by building a parse tree `Namespace` node from the `Namespace` token (as there isn't a `;` token). Allow this correspondence on errors. Also teach the diagnostics in this case to avoid the end-of-file token as that's almost always going to be a less meaningful location. Instead, we can point at the introducer which should at least be in the code that led to the error. --- .../parse/handle_decl_name_and_params.cpp | 8 ++- toolchain/parse/node_kind.def | 4 +- .../testdata/namespace/fail_incomplete.carbon | 59 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 toolchain/parse/testdata/namespace/fail_incomplete.carbon diff --git a/toolchain/parse/handle_decl_name_and_params.cpp b/toolchain/parse/handle_decl_name_and_params.cpp index 8d7c4f55fe8e..59045d6b333e 100644 --- a/toolchain/parse/handle_decl_name_and_params.cpp +++ b/toolchain/parse/handle_decl_name_and_params.cpp @@ -27,7 +27,13 @@ static auto HandleDeclNameAndParams(Context& context, State after_name) CARBON_DIAGNOSTIC(ExpectedDeclName, Error, "`{0}` introducer should be followed by a name.", Lex::TokenKind); - context.emitter().Emit(*context.position(), ExpectedDeclName, + Lex::Token location = *context.position(); + if (context.tokens().GetKind(location) == Lex::TokenKind::EndOfFile) { + // The end of file is often an especially unhelpful location. If that's + // the best we can do here, back up the location to the introducer itself. + location = state.token; + } + context.emitter().Emit(location, ExpectedDeclName, context.tokens().GetKind(state.token)); context.ReturnErrorOnState(); context.AddLeafNode(NodeKind::InvalidParse, *context.position(), diff --git a/toolchain/parse/node_kind.def b/toolchain/parse/node_kind.def index 19157ac014bd..0f712a06b15a 100644 --- a/toolchain/parse/node_kind.def +++ b/toolchain/parse/node_kind.def @@ -141,7 +141,9 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(LibrarySpecifier, 1, CARBON_TOKEN(Library)) // _external_: Name or QualifiedDecl // Namespace CARBON_PARSE_NODE_KIND_CHILD_COUNT(NamespaceStart, 0, CARBON_TOKEN(Namespace)) -CARBON_PARSE_NODE_KIND_CHILD_COUNT(Namespace, 2, CARBON_TOKEN(Semi)) +CARBON_PARSE_NODE_KIND_CHILD_COUNT(Namespace, 2, + CARBON_TOKEN(Semi) + CARBON_IF_ERROR(CARBON_TOKEN(Namespace))) // A code block: // CodeBlockStart diff --git a/toolchain/parse/testdata/namespace/fail_incomplete.carbon b/toolchain/parse/testdata/namespace/fail_incomplete.carbon new file mode 100644 index 000000000000..6455d9b13ef0 --- /dev/null +++ b/toolchain/parse/testdata/namespace/fail_incomplete.carbon @@ -0,0 +1,59 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:11: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace 123 +// CHECK:STDERR: ^ +namespace 123 + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:11: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace "" +// CHECK:STDERR: ^ +namespace "" + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:11: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace + +// CHECK:STDERR: ^ +namespace + + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:11: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace bool +// CHECK:STDERR: ^ +namespace bool + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:11: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace namespace namespace namespace namespace +// CHECK:STDERR: ^ +namespace namespace namespace namespace namespace + +// CHECK:STDERR: fail_incomplete.carbon:[[@LINE+3]]:1: ERROR: `namespace` introducer should be followed by a name. +// CHECK:STDERR: namespace +// CHECK:STDERR: ^ +namespace + +// CHECK:STDOUT: - filename: fail_incomplete.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: '123', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: '""', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: '+', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: 'bool', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: 'namespace', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, +// CHECK:STDOUT: {kind: 'InvalidParse', text: '', has_error: yes}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ]