From 6abc1299927860a3d433119c9632cf780ac79c7b Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Tue, 10 Nov 2026 00:15:22 +0100 Subject: rewrote number parsing: convert only completely parsed numbers * We are now buffering until the end of the number constant before converting it to an integer or float. * This way, the integer part of a floating point number does not necessarily have to fit into an teco_int_t -- floats can approximate very large numbers. When detecting a floating point number, we're parsing into floats from the very beginning. The behavior has been compared with Lua 5.4 and a test case has been added. * Also, integers constants are now checked for overflows. Otherwise, constants could be arbitrarily long and the result would be unpredictable. Operations on integers are still not checked for overflows, though, since you might want to rely on it, e.g. for overlapping integers. But that might actually be pretty useless, especially with signed integers, so we might want to change that in the future. * Having both overflow checks and increased floating point constant range is the reason we cannot practically immediately parse into a number, not even if we kept both an integer and float accumulator at the same time. * Also fixes a memory corruption issue that existed since 4aa2b717f9e050783b567540d9bb0193d692e3e9. You must emit undo tokens for fields in state machines only if ctx->parent.must_undo, i.e. if the state machine is not temporary as during a macro call. A test case has been added. --- src/error.h | 9 +++ src/number-commands.c | 166 +++++++++++++++++++++++++++++++++----------------- src/parser.c | 1 + src/parser.h | 10 ++- tests/testsuite.at | 11 ++++ 5 files changed, 135 insertions(+), 62 deletions(-) diff --git a/src/error.h b/src/error.h index 1306feb..19b707e 100644 --- a/src/error.h +++ b/src/error.h @@ -42,6 +42,7 @@ typedef enum { TECO_ERROR_SYNTAX, TECO_ERROR_MODIFIER, TECO_ERROR_ARGEXPECTED, + TECO_ERROR_RADIX, TECO_ERROR_LABEL, TECO_ERROR_CODEPOINT, TECO_ERROR_MOVE, @@ -94,6 +95,14 @@ teco_error_argexpected_set(GError **error, const gchar *cmd) "Argument expected for <%s>", cmd); } +static inline void +teco_error_radix_set(GError **error, gchar chr, teco_int_t radix) +{ + g_set_error(error, TECO_ERROR, TECO_ERROR_RADIX, + "Digit \"%c\" not allowed in radix %" TECO_INT_FORMAT, + chr, radix); +} + static inline void teco_error_label_set(GError **error, const gchar *name, gsize len) { diff --git a/src/number-commands.c b/src/number-commands.c index 27cf8ba..6ba2874 100644 --- a/src/number-commands.c +++ b/src/number-commands.c @@ -18,6 +18,8 @@ #include "config.h" #endif +#include + #include #include "sciteco.h" @@ -28,6 +30,7 @@ #include "parser.h" #include "interface.h" #include "expressions.h" +#include "string-utils.h" #include "core-commands.h" #include "number-commands.h" @@ -36,85 +39,136 @@ static teco_state_t teco_state_number; static gboolean teco_state_number_end_of_macro(teco_machine_main_t *ctx, GError **error) { - teco_undo_guint(ctx->number.radix) = 0; /* default */ - guint divisor = ctx->number.divisor; - teco_undo_guint(ctx->number.divisor) = 0; /* default */ + teco_int_t radix = ctx->number.radix; + if (ctx->parent.must_undo) + teco_undo_guint(ctx->number.radix); + ctx->number.radix = 0; /* default */ if (ctx->flags.mode > TECO_MODE_NORMAL) + /* ctx->number.value hasn't been updated */ return TRUE; - if (divisor) - /* floating point number */ - teco_expressions_push((teco_float_t)ctx->number.value / divisor); + if (!radix) + /* + * The ^R radix is only considered if we do not + * syntactically signify the radix (octal or hex constants). + * + * FIXME: We could just access qreg->number.i here since + * we can assume that "^R" is a plain register. + */ + teco_qreg_get_integer(ctx->qreg_table_locals->radix, &radix, NULL); + + gboolean is_float = teco_string_contains(ctx->number.value, '.'); + if (is_float) { + if (radix == 8 && g_str_has_prefix(ctx->number.value.data, "0.")) + /* otherwise 0.1 would be parsed in octal */ + radix = 10; + + teco_float_t divisor = 0, value = 0; + + for (const gchar *p = ctx->number.value.data; *p != '\0'; p++) { + if (*p == '.') { + divisor = 1; + continue; + } + + gchar chr = teco_ascii_toupper(*p); + gint digit = 'A' <= chr && chr <= 'F' ? chr-'A'+10 : chr-'0'; + if (digit >= radix) { + teco_error_radix_set(error, chr, radix); + return FALSE; + } + + if (divisor) + divisor *= radix; + value = value*radix + teco_num_sign*digit; + } + + teco_set_num_sign(1); + teco_expressions_push(value / divisor); + } else { + teco_int_t value = 0; + + for (const gchar *p = ctx->number.value.data; *p != '\0'; p++) { + gchar chr = teco_ascii_toupper(*p); + gint digit = 'A' <= chr && chr <= 'F' ? chr-'A'+10 : chr-'0'; + if (digit >= radix) { + teco_error_radix_set(error, chr, radix); + return FALSE; + } + + /* + * By taking the teco_num_sign into account we can + * properly check for underflows. + * Otherwise MININT (-9223372036854775808) would overflow. + * + * NOTE: Glib 2.48 has g_size_checked_add(). + */ + if (__builtin_mul_overflow(value, radix, &value) || + __builtin_add_overflow(value, teco_num_sign*digit, &value)) { + g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED, + "Integer constant overflow"); + return FALSE; + } + } + + teco_set_num_sign(1); + teco_expressions_push(value); + } + + if (ctx->parent.must_undo) + teco_undo_string_own(ctx->number.value); else - teco_expressions_push(ctx->number.value); - teco_undo_int(ctx->number.value) = 0; + teco_string_clear(&ctx->number.value); + memset(&ctx->number.value, 0, sizeof(ctx->number.value)); + return TRUE; } teco_state_t * teco_state_number_input(teco_machine_main_t *ctx, gunichar chr, GError **error) { - /* - * If you have several dots, the second one will push the current buffer position. - * E.g. 23.0. is equivalent to 23.0,. - */ - if (chr == '.' && !ctx->number.divisor) { - /* floating point number */ - if (ctx->number.radix == 8 && ctx->number.value == 0) - /* otherwise 0.1 would be parsed in octal */ - teco_undo_guint(ctx->number.radix) = 10; - teco_undo_guint(ctx->number.divisor) = 1; - return &teco_state_number; - } - - /* - * Hexadecimal constants (@[0-9][0-9a-zA-Z]*) are terminated by any non-hexadecimal character. - * This means you may have to append a space or braces to stop parsing if the next - * command would begin with such a letter (e.g. `@0ff D`). - */ - gboolean end_of_number = chr <= 0xFF && (ctx->number.radix == 16 ? !g_ascii_isxdigit(chr) - : !g_ascii_isdigit(chr)); + gboolean end_of_number; + if (chr == '.') + /* + * If you have several dots, the second one will push the current buffer position. + * E.g. 23.0. is equivalent to 23.0,. + */ + end_of_number = teco_string_contains(ctx->number.value, '.'); + else + /* + * Hexadecimal constants (@[0-9][0-9a-zA-Z]*) are terminated by any non-hexadecimal character. + * This means you may have to append a space or braces to stop parsing if the next + * command would begin with such a letter (e.g. `@0ff D`). + */ + end_of_number = chr > 0xFF || (ctx->number.radix == 16 ? !g_ascii_isxdigit(chr) + : !g_ascii_isdigit(chr)); if (end_of_number) { - teco_state_number_end_of_macro(ctx, error); + if (!teco_state_number_end_of_macro(ctx, error)) + return NULL; return teco_state_start_input(ctx, chr, error); } if (!ctx->number.radix) { - if (teco_machine_main_eval_at(ctx)) + if (teco_machine_main_eval_at(ctx)) { /* @[0-9] starts a hexadecimal constant */ - teco_undo_guint(ctx->number.radix) = 16; - else if (!ctx->number.value && chr == '0') + if (ctx->parent.must_undo) + teco_undo_guint(ctx->number.radix); + ctx->number.radix = 16; + } else if (!ctx->number.value.len && chr == '0') { /* a leading zero starts an octal constant */ - teco_undo_guint(ctx->number.radix) = 8; + if (ctx->parent.must_undo) + teco_undo_guint(ctx->number.radix); + ctx->number.radix = 8; + } } if (ctx->flags.mode > TECO_MODE_NORMAL) return &teco_state_number; - teco_int_t radix = ctx->number.radix; - if (!radix) - /* - * The ^R radix is only considered if we do not - * syntactically signify the radix (octal or hex constants). - * - * FIXME: We could just access qreg->number.i here since - * we can assume that "^R" is a plain register. - */ - teco_qreg_get_integer(ctx->qreg_table_locals->radix, &radix, NULL); - - chr = teco_ascii_toupper(chr); - guint digit = 'A' <= chr && chr <= 'F' ? chr-'A'+10 : chr-'0'; - if (digit >= radix) { - g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED, - "Digit \"%c\" not allowed in radix %" TECO_INT_FORMAT, - (gchar)chr, radix); - return NULL; - } - - teco_undo_int(ctx->number.value) = ctx->number.value*radix + digit; - if (ctx->number.divisor) - teco_undo_guint(ctx->number.divisor) *= radix; + if (ctx->parent.must_undo) + undo__teco_string_truncate(&ctx->number.value, ctx->number.value.len); + teco_string_append_wc(&ctx->number.value, chr); return &teco_state_number; } diff --git a/src/parser.c b/src/parser.c index 231ad3e..28f6fd8 100644 --- a/src/parser.c +++ b/src/parser.c @@ -402,6 +402,7 @@ void teco_machine_main_clear(teco_machine_main_t *ctx) { teco_goto_table_clear(&ctx->goto_table); + teco_string_clear(&ctx->number.value); teco_string_clear(&ctx->expectstring.string); teco_machine_stringbuilding_clear(&ctx->expectstring.machine); teco_string_clear(&ctx->goto_label); diff --git a/src/parser.h b/src/parser.h index c134d94..8303a2d 100644 --- a/src/parser.h +++ b/src/parser.h @@ -441,14 +441,12 @@ typedef struct { * In case of 0, "^R" determines the radix (10 by default). */ guint radix; - /** accumulator for parsing numbers */ - teco_int_t value; /** - * if not null, value/divisor is the floating point number - * FIXME: Perhaps roll this into a bitfield with radix. - * The entire teco_machine_number_t could be undone. + * Accumulator for integer constants. + * It is not parsed immediately, so we can handle integer + * and float constants differently. */ - guint divisor; + teco_string_t value; } teco_machine_number_t; /** diff --git a/tests/testsuite.at b/tests/testsuite.at index bc5c6da..73d4440 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -49,6 +49,9 @@ TE_CHECK([[23:=]], 0, [23], ignore) TE_CHECK([[1 2= =]], 0, ignore, ignore) TE_CHECK([[033-27"N(0/0)']], 0, ignore, ignore) TE_CHECK([[@0ff-255"N(0/0)']], 0, ignore, ignore) +# Overflows should be catched +TE_CHECK([[@0ffffffffffffffffff$]], 1, ignore, ignore) +TE_CHECK([[-@0ffffffffffffffffff$]], 1, ignore, ignore) # floating point numbers # NOTE: %q does not change the register's number type. TE_CHECK([[1.Ua 0.1%a:=]], 0, [1.1], ignore) @@ -56,6 +59,8 @@ TE_CHECK([[1Ua 0.1%a:=]], 0, [1], ignore) TE_CHECK([[23.04:=]], 0, [23.04], ignore) TE_CHECK([[0.04:=]], 0, [0.04], ignore) TE_CHECK([[23.04#0:=]], 0, [23], ignore) +AT_SKIP_IF([test $TECO_NUMBER -lt 64]) +TE_CHECK([[11111111111111111111.1:=]], 0, [11111111111111110656], ignore) AT_CLEANUP AT_SETUP([Number stack]) @@ -733,6 +738,12 @@ TE_CHECK_CMDLINE([[[.a@^Um{].b}Mm{-2D}]], 0, ignore, stderr) AT_FAIL_IF([$GREP "^Error:" stderr]) AT_CLEANUP +AT_SETUP([Rub out numbers in macro calls]) +# This was causing memory corruptions, that would at least show up under Valgrind. +TE_CHECK_CMDLINE([[@^Um{023$}Mm{-2D}]], 0, ignore, stderr) +AT_FAIL_IF([$GREP "^Error:" stderr]) +AT_CLEANUP + AT_SETUP([Searches from macro calls]) TE_CHECK_CMDLINE([[@^Um{:@S/XXX/} :Mm"S(0/0)' Mm"S(0/0)']], 0, ignore, stderr) AT_FAIL_IF([$GREP "^Error:" stderr]) -- cgit v1.2.3