diff --git a/CHANGELOG.md b/CHANGELOG.md index c14f6a7c1d..839fd6d208 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ### Added -- `pm_options_stack_budget_set` was added to the C API. It sets the number of bytes of C stack the parser may use before reporting that the source is nested too deeply, instead of overflowing the stack. `SIZE_MAX` means no budget, which is the default. The nesting depth limit alone assumes a stack of roughly 8MB, so embedders that parse on a smaller stack, such as a thread's or a fiber's, should pass a budget. +- `pm_options_stack_budget_set` was added to the C API. It sets the number of bytes of C stack the parser may use before reporting a `stack_exhausted` error, instead of overflowing the stack. `SIZE_MAX` means no budget, which is the default. The nesting depth limit alone assumes a stack of roughly 8MB, so embedders that parse on a smaller stack, such as a thread's or a fiber's, should pass a budget. +- A new `stack` error level, for errors that should raise `SystemStackError`. Its only error is `stack_exhausted`, reported once per parse when the stack budget is used up. It is separate from `nesting_too_deep`, which stays a syntax error, because the same source may parse with more stack. When errors are formatted, a `stack_exhausted` error takes precedence over the syntax errors around it, as argument and load errors do. The level is `:stack` in Ruby, `"stack"` in JavaScript, `ERROR_STACK` in Java, and `PM_ERROR_LEVEL_STACK` in C and Rust. ### Changed diff --git a/config.yml b/config.yml index 14ea82f405..217b16e763 100644 --- a/config.yml +++ b/config.yml @@ -262,6 +262,7 @@ errors: - RETURN_INVALID - SCRIPT_NOT_FOUND - SINGLETON_FOR_LITERALS + - STACK_EXHAUSTED - STATEMENT_ALIAS - STATEMENT_POSTEXE_END - STATEMENT_PREEXE_BEGIN diff --git a/docs/serialization.md b/docs/serialization.md index cbe7746f40..63bd5a2b64 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -69,7 +69,7 @@ Only nodes that own comments have an attachment, and there are none unless the ` | varuint | type | | string | error message (ASCII-only characters) | | location | the location in the source this error applies to | -| `1` | the level of the error: `0` for `fatal`, `1` for `argument`, `2` for `load` | +| `1` | the level of the error: `0` for `syntax`, `1` for `argument`, `2` for `load`, `3` for `stack` | ### warning diff --git a/ext/prism/extension.c b/ext/prism/extension.c index 78d60731a7..20c74e0e40 100644 --- a/ext/prism/extension.c +++ b/ext/prism/extension.c @@ -493,6 +493,10 @@ check_raise_error_option(pm_parser_t *parser, const pm_options_t *options, rb_en rb_ivar_set(error, rb_intern_const("@path"), Qnil); break; } + case PM_ERROR_LEVEL_STACK: { + error = rb_exc_new_str(rb_eSysStackError, message); + break; + } } return result_err(error); @@ -716,6 +720,9 @@ parser_error(const pm_diagnostic_t *diagnostic, rb_encoding *encoding, VALUE sou case PM_ERROR_LEVEL_LOAD: level = ID2SYM(rb_intern("load")); break; + case PM_ERROR_LEVEL_STACK: + level = ID2SYM(rb_intern("stack")); + break; default: rb_raise(rb_eRuntimeError, "Unknown level: %" PRIu8, error_level); } diff --git a/include/prism/diagnostic.h b/include/prism/diagnostic.h index 202a3e782a..560d61ef4d 100644 --- a/include/prism/diagnostic.h +++ b/include/prism/diagnostic.h @@ -32,7 +32,14 @@ typedef enum { PM_ERROR_LEVEL_ARGUMENT = 1, /** For errors that should raise a load error. */ - PM_ERROR_LEVEL_LOAD = 2 + PM_ERROR_LEVEL_LOAD = 2, + + /** + * For errors that should raise a system stack error. These mean that the + * parser ran out of the stack it was given, not that the source is + * invalid, so the same source may parse with more stack. + */ + PM_ERROR_LEVEL_STACK = 3 } pm_error_level_t; /** diff --git a/include/prism/errors_format.h b/include/prism/errors_format.h index 9706c2b57c..b94940e568 100644 --- a/include/prism/errors_format.h +++ b/include/prism/errors_format.h @@ -48,9 +48,10 @@ typedef enum { * @param buffer The buffer to format the errors into. * @param format_type The type of formatting to use when formatting the errors. * @returns The error level of the error with the highest precedence that was - * formatted. In practice this means it will be the argument or load error - * level if any were found, otherwise it will be the syntax error level. - * Callers should raise appropriate error types based on the returned level. + * formatted. In practice this means it will be the level of the first + * argument, load, or stack error if any were found, otherwise it will be the + * syntax error level. Callers should raise appropriate error types based on + * the returned level. */ PRISM_EXPORTED_FUNCTION pm_error_level_t pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_format_type_t format_type) PRISM_NONNULL(1, 2); diff --git a/include/prism/internal/options.h b/include/prism/internal/options.h index 0c98dba849..71fa1a6b95 100644 --- a/include/prism/internal/options.h +++ b/include/prism/internal/options.h @@ -78,9 +78,9 @@ struct pm_options_t { /* * The number of bytes of C stack that the parser may use, measured from * where pm_parse is called. When the recursive descent goes past it, the - * parser reports that the input is nested too deeply instead of - * overflowing the stack. SIZE_MAX means no budget was given, in which case - * only PRISM_DEPTH_MAXIMUM bounds the recursion. + * parser reports that the stack is exhausted instead of overflowing it. + * SIZE_MAX means no budget was given, in which case only + * PRISM_DEPTH_MAXIMUM bounds the recursion. */ size_t stack_budget; diff --git a/include/prism/internal/parser.h b/include/prism/internal/parser.h index 60ae79d457..42f3a437c0 100644 --- a/include/prism/internal/parser.h +++ b/include/prism/internal/parser.h @@ -889,8 +889,8 @@ struct pm_parser_t { /* * The number of bytes of C stack below stack_base that the recursive - * descent may use before it reports that the input is nested too deeply. - * SIZE_MAX when no budget was given, so the check never fires. + * descent may use before it reports that the stack is exhausted. SIZE_MAX + * when no budget was given, so the check never fires. */ size_t stack_budget; @@ -986,6 +986,13 @@ struct pm_parser_t { */ bool warn_mismatched_indentation; + /* + * Whether the stack exhausted error has been reported. Once the budget is + * used up, every recursion that unwinds past it would report the error + * again, so it is only reported the first time. + */ + bool stack_exhausted_reported; + #if defined(PRISM_HAS_NEON) || defined(PRISM_HAS_SSE2) || defined(PRISM_HAS_SWAR) /* * Cached lookup tables for pm_strpbrk's SIMD fast path. Avoids rebuilding @@ -1041,4 +1048,12 @@ pm_parser_stack_exhausted(const pm_parser_t *parser) { return used > parser->stack_budget; } +/* + * Reports that the stack budget has been used up, at the given location. This + * is a separate error from the nesting depth limit, with its own error level, + * because the source may be valid and parse with more stack. It is reported + * once per parse. + */ +void pm_parser_err_stack_exhausted(pm_parser_t *parser, uint32_t start, uint32_t length); + #endif diff --git a/include/prism/options.h b/include/prism/options.h index 19f520854c..58062061c3 100644 --- a/include/prism/options.h +++ b/include/prism/options.h @@ -291,14 +291,16 @@ PRISM_EXPORTED_FUNCTION void pm_options_raise_error_set(pm_options_t *options, u /** * Set the stack_budget option on the given options struct. This is the number * of bytes of C stack that the parser may use, measured from where pm_parse is - * called. Deeply nested input that would go past it is reported as nested too - * deeply instead of overflowing the stack. Callers should pass less than the - * stack they actually have left, since the parser checks the budget between - * frames rather than inside every one, and anything that walks the tree - * afterward recurses on the same stack. A budget of 0 leaves no room for any - * expression, so every one is reported as nested too deeply. A budget of - * SIZE_MAX (the default) disables the check, leaving PRISM_DEPTH_MAXIMUM as the - * only bound, which assumes a stack of roughly 8MB. + * called. Deeply nested input that would go past it produces a + * PM_ERR_STACK_EXHAUSTED error instead of overflowing the stack. That error + * has PM_ERROR_LEVEL_STACK rather than PM_ERROR_LEVEL_SYNTAX, since the same + * source may parse with a larger budget, and it is reported once per parse. + * Callers should pass less than the stack they actually have left, since the + * parser checks the budget between frames rather than inside every one, and + * anything that walks the tree afterward recurses on the same stack. A budget + * of 0 leaves no room for any expression. A budget of SIZE_MAX (the default) + * disables the check, leaving PRISM_DEPTH_MAXIMUM as the only bound, which + * assumes a stack of roughly 8MB. * * @param options The options struct to set the stack_budget value on. * @param stack_budget The stack_budget value to set, in bytes, or SIZE_MAX for diff --git a/java/api/src/main/java/org/ruby_lang/prism/ParseResult.java b/java/api/src/main/java/org/ruby_lang/prism/ParseResult.java index 7931327b9a..38a66931cd 100644 --- a/java/api/src/main/java/org/ruby_lang/prism/ParseResult.java +++ b/java/api/src/main/java/org/ruby_lang/prism/ParseResult.java @@ -22,6 +22,9 @@ public enum ErrorLevel { /** For errors that should raise LoadError. */ ERROR_LOAD, + + /** For errors that should raise SystemStackError. */ + ERROR_STACK, } public static ErrorLevel[] ERROR_LEVELS = ErrorLevel.values(); diff --git a/lib/prism/ffi.rb b/lib/prism/ffi.rb index d2cee03684..f3158cbb5d 100644 --- a/lib/prism/ffi.rb +++ b/lib/prism/ffi.rb @@ -537,6 +537,8 @@ def raise_error(string, options, format_type) # :nodoc: error = LoadError.new(message.force_encoding(Encoding.find("locale"))) error.instance_variable_set(:@path, nil) raise error + when 3 # stack + raise SystemStackError, message.force_encoding(encoding_name) else raise "Unknown error level: #{level}" end diff --git a/rust/ruby-prism-sys/src/bindings.rs b/rust/ruby-prism-sys/src/bindings.rs index a8c2092668..aae2b8d4fd 100644 --- a/rust/ruby-prism-sys/src/bindings.rs +++ b/rust/ruby-prism-sys/src/bindings.rs @@ -10474,6 +10474,11 @@ pub enum pm_error_level_t { /** For errors that should raise a load error. */ PM_ERROR_LEVEL_LOAD = 2, + /** For errors that should raise a system stack error. These mean that the + parser ran out of the stack it was given, not that the source is + invalid, so the same source may parse with more stack. +*/ + PM_ERROR_LEVEL_STACK = 3, } #[repr(u32)] #[non_exhaustive] diff --git a/src/errors_format.c b/src/errors_format.c index 645a997088..595580a136 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -581,6 +581,23 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma xfree_sized(rich_errors, rich_errors_size); return PM_ERROR_LEVEL_LOAD; } + case PM_ERROR_LEVEL_STACK: { + /* If we get a stack error, we want to format it and return it + * immediately. Once the stack is exhausted, the syntax errors + * around it come from recovering from that point rather than from + * the source, so they are left out. It should include a snippet of + * the source if possible. */ + pm_rich_error_init(parser, rich_errors, diagnostic, &location); + pm_error_append_summary(parser, buffer, rich_errors[0].line_start, pm_diagnostic_message(diagnostic)); + + if (rich_errors_are_utf8(parser, 1, rich_errors)) { + pm_buffer_append_byte(buffer, '\n'); + pm_rich_errors_format(parser, buffer, format_type, 1, rich_errors, false); + } + + xfree_sized(rich_errors, rich_errors_size); + return PM_ERROR_LEVEL_STACK; + } } } diff --git a/src/prism.c b/src/prism.c index e5621e0ad8..b1fc48724e 100644 --- a/src/prism.c +++ b/src/prism.c @@ -550,6 +550,16 @@ pm_parser_err_current(pm_parser_t *parser, pm_diagnostic_id_t diag_id) { pm_parser_err_token(parser, &parser->current, diag_id); } +/** + * Reports that the stack budget has been used up, at the given location. + */ +void +pm_parser_err_stack_exhausted(pm_parser_t *parser, uint32_t start, uint32_t length) { + if (parser->stack_exhausted_reported) return; + parser->stack_exhausted_reported = true; + pm_parser_err(parser, start, length, PM_ERR_STACK_EXHAUSTED); +} + /** * Append an error to the list of errors on the parser using the location of the * previous token. @@ -17850,7 +17860,12 @@ parse_pattern(pm_parser_t *parser, pm_constant_id_set_t *captures, uint8_t flags /* Nested patterns recurse back into parse_pattern without passing through * parse_expression, so this cycle needs its own depth check. */ if (PRISM_UNLIKELY(depth >= PRISM_DEPTH_MAXIMUM || pm_parser_stack_exhausted(parser))) { - pm_parser_err_current(parser, PM_ERR_NESTING_TOO_DEEP); + if (depth >= PRISM_DEPTH_MAXIMUM) { + pm_parser_err_current(parser, PM_ERR_NESTING_TOO_DEEP); + } else { + pm_parser_err_stack_exhausted(parser, PM_TOKEN_START(parser, &parser->current), PM_TOKEN_LENGTH(&parser->current)); + } + return UP(pm_error_recovery_node_create(parser, PM_TOKEN_START(parser, &parser->current), PM_TOKEN_LENGTH(&parser->current))); } @@ -22819,7 +22834,12 @@ parse_expression_terminator(pm_parser_t *parser, pm_node_t *node) { static pm_node_t * parse_expression(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t flags, pm_diagnostic_id_t diag_id, uint16_t depth) { if (PRISM_UNLIKELY(depth >= PRISM_DEPTH_MAXIMUM || pm_parser_stack_exhausted(parser))) { - pm_parser_err_current(parser, PM_ERR_NESTING_TOO_DEEP); + if (depth >= PRISM_DEPTH_MAXIMUM) { + pm_parser_err_current(parser, PM_ERR_NESTING_TOO_DEEP); + } else { + pm_parser_err_stack_exhausted(parser, PM_TOKEN_START(parser, &parser->current), PM_TOKEN_LENGTH(&parser->current)); + } + return UP(pm_error_recovery_node_create(parser, PM_TOKEN_START(parser, &parser->current), PM_TOKEN_LENGTH(&parser->current))); } @@ -23519,6 +23539,7 @@ pm_parse_err_is_fatal(pm_diagnostic_id_t diag_id) { case PM_ERR_INVALID_NUMBER_UNDERSCORE_TRAILING: case PM_ERR_NO_LOCAL_VARIABLE: case PM_ERR_PARAMETER_ORDER: + case PM_ERR_STACK_EXHAUSTED: case PM_ERR_STATEMENT_UNDEF: case PM_ERR_VOID_EXPRESSION: return true; diff --git a/src/regexp.c b/src/regexp.c index 013bb23760..ed8b0af5d3 100644 --- a/src/regexp.c +++ b/src/regexp.c @@ -139,6 +139,25 @@ pm_regexp_parse_error(pm_regexp_parser_t *regexp_parser, const uint8_t *start, c } } +/** + * Report that the stack budget was used up while parsing the regular + * expression. Unlike other errors, this is reported even when report_errors is + * not set, because giving up early there would silently drop the named + * captures that the rest of the regular expression defines. In that case start + * and end do not point into the original source, so the error is reported at + * the token that the parser is on instead. + */ +static void +pm_regexp_parse_stack_exhausted(pm_regexp_parser_t *regexp_parser) { + pm_parser_t *parser = regexp_parser->parser; + + if (regexp_parser->report_errors) { + pm_parser_err_stack_exhausted(parser, (uint32_t) (regexp_parser->start - parser->start), (uint32_t) (regexp_parser->end - regexp_parser->start)); + } else { + pm_parser_err_stack_exhausted(parser, (uint32_t) (parser->current.start - parser->start), (uint32_t) (parser->current.end - parser->current.start)); + } +} + /** * Append a formatted syntax error to the parser's error list, following the * same rules as pm_regexp_parse_error. This is a macro because we need @@ -714,11 +733,16 @@ pm_regexp_parse_hex_escape(pm_regexp_parser_t *parser) { */ static bool pm_regexp_parse_backslash_escape(pm_regexp_parser_t *parser, uint16_t depth) { - if (depth >= PM_REGEXP_PARSE_DEPTH_MAX || pm_parser_stack_exhausted(parser->parser)) { + if (depth >= PM_REGEXP_PARSE_DEPTH_MAX) { pm_regexp_parse_error(parser, parser->start, parser->end, "parse depth limit over"); return false; } + if (pm_parser_stack_exhausted(parser->parser)) { + pm_regexp_parse_stack_exhausted(parser); + return false; + } + if (pm_regexp_char_is_eof(parser)) return true; switch (*parser->cursor) { @@ -846,11 +870,16 @@ pm_regexp_parse_character_set(pm_regexp_parser_t *parser, uint16_t depth) { */ static bool pm_regexp_parse_lbracket(pm_regexp_parser_t *parser, uint16_t depth) { - if (depth >= PM_REGEXP_PARSE_DEPTH_MAX || pm_parser_stack_exhausted(parser->parser)) { + if (depth >= PM_REGEXP_PARSE_DEPTH_MAX) { pm_regexp_parse_error(parser, parser->start, parser->end, "parse depth limit over"); return false; } + if (pm_parser_stack_exhausted(parser->parser)) { + pm_regexp_parse_stack_exhausted(parser); + return false; + } + if ((parser->cursor < parser->end) && parser->cursor[0] == ']') { parser->cursor++; pm_regexp_parse_error(parser, parser->cursor - 1, parser->cursor, "empty char-class"); @@ -1011,7 +1040,7 @@ pm_regexp_parse_group(pm_regexp_parser_t *parser, uint16_t depth) { * stops that function from being inlined, which adds a frame to every * level of nesting. */ if (pm_parser_stack_exhausted(parser->parser)) { - pm_regexp_parse_error(parser, parser->start, parser->end, "parse depth limit over"); + pm_regexp_parse_stack_exhausted(parser); return false; } diff --git a/templates/javascript/src/deserialize.js.erb b/templates/javascript/src/deserialize.js.erb index d1fdbf507c..55f74907b8 100644 --- a/templates/javascript/src/deserialize.js.erb +++ b/templates/javascript/src/deserialize.js.erb @@ -244,7 +244,7 @@ export class ParseResult { } } -const errorLevels = ["syntax", "argument", "load"]; +const errorLevels = ["syntax", "argument", "load", "stack"]; const errorTypes = [ <%- errors.each do |error| -%> "<%= error.name.downcase %>", diff --git a/templates/lib/prism/serialize.rb.erb b/templates/lib/prism/serialize.rb.erb index 32afb2fd29..aebf8a802f 100644 --- a/templates/lib/prism/serialize.rb.erb +++ b/templates/lib/prism/serialize.rb.erb @@ -412,6 +412,8 @@ module Prism :argument when 2 :load + when 3 + :stack else raise "Unknown level: #{level}" end diff --git a/templates/src/diagnostic.c.erb b/templates/src/diagnostic.c.erb index 7283411186..50635ab77c 100644 --- a/templates/src/diagnostic.c.erb +++ b/templates/src/diagnostic.c.erb @@ -82,6 +82,7 @@ typedef struct { * * `PM_ERROR_LEVEL_SYNTAX` - Errors that should raise SyntaxError. * * `PM_ERROR_LEVEL_ARGUMENT` - Errors that should raise ArgumentError. * * `PM_ERROR_LEVEL_LOAD` - Errors that should raise LoadError. + * * `PM_ERROR_LEVEL_STACK` - Errors that should raise SystemStackError. * * For warnings, they are: * @@ -98,6 +99,9 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = { /* Errors that should raise load errors */ [PM_ERR_SCRIPT_NOT_FOUND] = { "no Ruby script found in input", PM_ERROR_LEVEL_LOAD }, + /* Errors that should raise system stack errors */ + [PM_ERR_STACK_EXHAUSTED] = { "stack level too deep", PM_ERROR_LEVEL_STACK }, + /* Errors that should raise syntax errors */ [PM_ERR_ALIAS_ARGUMENT] = { "invalid argument being passed to `alias`; expected a bare word, symbol, constant, or global variable", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_ALIAS_ARGUMENT_NUMBERED_REFERENCE] = { "invalid argument being passed to `alias`; can't make alias for the number variables", PM_ERROR_LEVEL_SYNTAX },