From 628c73d984fd7663607cc3fd9368f809855906fd Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Sun, 13 Apr 2025 00:40:37 +0300 Subject: fixed undoing bitfields on Windows * It turns out that `bool` (_Bool) in bitfields may cause padding to the next 32-bit word. This was only observed on MinGW. I am not entirely sure why, although the C standard does not guarantee much with regard to bitfield memory layout and there are 64-bit available due to passing anyway. Actually, they could also be layed out in a different order. * I am now consistently using guint instead of `bool` in bitfields to prevent any potential surprises. * The way that guint was aliased with bitfield structs for undoing teco_machine_main_t and teco_machine_qregspec_t flags was therefore insecure. It was not guaranteed that the __flags field really "captures" all of the bit field. Even with `guint v : 1` fields, this was not guaranteed. We would have required a static assertion for robustness. Alternatively, we could have declared a `gsize __flags` variable as well. This __should__ be safe since gsize should always be pointer sized and correspond to the platform's alignment. However, it's also not 100% guaranteed. Using classic ANSI C enums with bit operations to encode multiple fields and flags into a single integer also doesn't look very attractive. * Instead, we now define scalar types with their own teco_undo_push() shortcuts for the bitfield structs. This is in one way simpler and much more robust, but on the other hand complicates access to the flag variables. * It's a good question whether a `struct __attribute__((packed))` bitfield with guint fields would be a reliable replacement for flag enums, that are communicated with the "outside" (TECO) world. I am not going to risk it until GCC gives any guarantees, though. For the time being, bitfields are only used internally where the concrete memory layout (bit positions) is not crucial. * This fixes the test suite and therefore probably CI and nightly builds on Windows. * Test case: Rub out `@I//` or `@Xq` until before the `@`. The parser doesn't know that `@` is still set and allows all sorts of commands where `@` should be forbidden. * It's unknown how long this has been broken on Windows - quite possibly since v2.0. --- src/parser.h | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) (limited to 'src/parser.h') diff --git a/src/parser.h b/src/parser.h index 050467c..5477150 100644 --- a/src/parser.h +++ b/src/parser.h @@ -16,8 +16,6 @@ */ #pragma once -#include - #include #include @@ -52,7 +50,7 @@ typedef struct { * a signed integer, it's ok steal one * bit for the pass_through flag. */ - bool pass_through : 1; + guint pass_through : 1; } teco_loop_context_t; extern GArray *teco_loop_stack; @@ -77,8 +75,8 @@ void undo__remove_index__teco_loop_stack(guint); * FIXME: Maybe use TECO_DECLARE_VTABLE_METHOD()? */ typedef const struct { - bool string_building : 1; - bool last : 1; + guint string_building : 1; + guint last : 1; /** * Called repeatedly to process chunks of input and give interactive feedback. @@ -206,7 +204,7 @@ struct teco_state_t { * This is separate of TECO_KEYMACRO_MASK_START which is set * only in the main machine's start states. */ - bool is_start : 1; + guint is_start : 1; /** * Key macro mask. * This is not a bitmask since it is compared with values set @@ -476,25 +474,17 @@ struct teco_machine_main_t { /** Program counter, i.e. pointer to the next character in the current macro frame */ gsize macro_pc; - /** - * Aliases bitfield with an integer. - * This allows teco_undo_guint(__flags), - * while still supporting easy-to-access flags. - */ - union { - struct { - teco_mode_t mode : 8; - - /** number of `:`-modifiers detected */ - guint modifier_colon : 2; - /** - * Whether the `@`-modifier has been detected. - * This is tracked even in parse-only mode. - */ - bool modifier_at : 1; - }; - guint __flags; - }; + struct teco_machine_main_flags_t { + teco_mode_t mode : 8; + + /** number of `:`-modifiers detected */ + guint modifier_colon : 2; + /** + * Whether the `@`-modifier has been detected. + * This is tracked even in parse-only mode. + */ + guint modifier_at : 1; + } flags; /** The nesting level of braces */ guint brace_level; @@ -529,6 +519,12 @@ struct teco_machine_main_t { }; }; +typedef struct teco_machine_main_flags_t teco_machine_main_flags_t; +TECO_DECLARE_UNDO_SCALAR(teco_machine_main_flags_t); + +#define teco_undo_flags(VAR) \ + (*teco_undo_object_teco_machine_main_flags_t_push(&(VAR))) + void teco_machine_main_init(teco_machine_main_t *ctx, teco_qreg_table_t *qreg_table_locals, gboolean must_undo); @@ -564,7 +560,7 @@ typedef const struct { * Since `@` has syntactic significance, * it is checked even in parse-only mode. */ - bool modifier_at : 1; + guint modifier_at : 1; } teco_machine_main_transition_t; /* -- cgit v1.2.3