Conversation
Collaborator
Author
|
@tonycoz is there any chance you can poke at the mingw failures? I suspect that they are unrelated to this patch and instead are related to an interaction between my recent TRIE change, and @richardleach's patch to flatten alternations when possible. I saw similar errors when i was working on the trie patch, which I thought I resolved, but this looks just the same. I am not sure why they didnt show up in the tests for that patch. The reason I think its unrelated to this patch is the error happens on a pattern that was added to test the trie's interaction with the alternation flattening and the pattern doesnt involve any char-classes, or unicode stuff at all. Im going to double check with asan checks on the linux code, but if you could poke on windows id be really grateful. Thanks! |
demerphq
added a commit
that referenced
this pull request
Sep 14, 2026
Fix intermittent segfaults from the trie logic, as reported in #24816, and discussed in #24814. The list compiler previously recorded a slot after a sparse frame in lasttrans without always allocating or clearing it. A lookup could therefore treat uninitialized transition data as belonging to its state and use an arbitrary next state. This issue affected all platforms but was most prominently visible on Windows and Cygwin. (Historically Windows realloc is not as good as *nix realloc.) As part of fixing this, I reworked some of the logic to be more robust and ensure we never see this kind of issue again. We give the root state the complete octet frame at slots 1 through 256. Other states can reuse unoccupied root slots. Each state base maps octet zero to an offset not less than 1, and we ensure there are enough valid null transitions at the end, so we never read a bogus transition and do not need to do bounds checks either. While working on this I noticed we had some logic left over from the old implementation that still used the concept of "charid", which confused matters. This was removed, and we now have simpler runtime logic to do transition lookups. I also added code to validate the state and transition indexes under DEBUG_r. Move that block outside DEBUG_r when validation is wanted in every full DEBUGGING build. Thus, compilation under C<use re 'debug'> also exercises the transition validation, so if you suspect a related bug try the pattern under that pragma. Added support to regexp.t to allow the developer to select an inclusive range of re_tests input lines while retaining its original TAP numbering. This makes the triggering cases practical to stress independently. The re_tests lines 2221 through 2222 also pass with 100,000 iterations, including under ASAN with a refcounted stack (PERL_RC_STACK).
demerphq
added a commit
that referenced
this pull request
Sep 14, 2026
Fix intermittent segfaults from the trie logic, as reported in #24816, and discussed in #24814. The list compiler previously recorded a slot after a sparse frame in lasttrans without always allocating or clearing it. A lookup could therefore treat uninitialized transition data as belonging to its state and use an arbitrary next state. This issue affected all platforms but was most prominently visible on Windows and Cygwin. (Historically Windows realloc is not as good as *nix realloc.) As part of fixing this, I reworked some of the logic to be more robust and ensure we never see this kind of issue again. We give the root state the complete octet frame at slots 1 through 256. Other states can reuse unoccupied root slots. Each state base maps octet zero to an offset not less than 1, and we ensure there are enough valid null transitions at the end, so we never read a bogus transition and do not need to do bounds checks either. While working on this I noticed we had some logic left over from the old implementation that still used the concept of "charid", which confused matters. This was removed, and we now have simpler runtime logic to do transition lookups. I also added code to validate the state and transition indexes under DEBUG_r. Move that block outside DEBUG_r when validation is wanted in every full DEBUGGING build. Thus, compilation under C<use re 'debug'> also exercises the transition validation, so if you suspect a related bug try the pattern under that pragma. Added support to regexp.t to allow the developer to select an inclusive range of re_tests input lines while retaining its original TAP numbering. This makes the triggering cases practical to stress independently. The re_tests lines 2221 through 2222 also pass with 100,000 iterations, including under ASAN with a refcounted stack (PERL_RC_STACK).
demerphq
added a commit
that referenced
this pull request
Sep 14, 2026
Fix intermittent segfaults from the trie logic, as reported in #24816, and discussed in #24814. The list compiler previously recorded a slot after a sparse frame in lasttrans without always allocating or clearing it. A lookup could therefore treat uninitialized transition data as belonging to its state and use an arbitrary next state. This issue affected all platforms but was most prominently visible on Windows and Cygwin. (Historically Windows realloc is not as good as *nix realloc.) As part of fixing this, I reworked some of the logic to be more robust and ensure we never see this kind of issue again. We give the root state the complete octet frame at slots 1 through 256. Other states can reuse unoccupied root slots. Each state base maps octet zero to an offset not less than 1, and we ensure there are enough valid null transitions at the end, so we never read a bogus transition and do not need to do bounds checks either. While working on this I noticed we had some logic left over from the old implementation that still used the concept of "charid", which confused matters. This was removed, and we now have simpler runtime logic to do transition lookups. I also added code to validate the state and transition indexes under DEBUG_r. Move that block outside DEBUG_r when validation is wanted in every full DEBUGGING build. Thus, compilation under C<use re 'debug'> also exercises the transition validation, so if you suspect a related bug try the pattern under that pragma. Added support to regexp.t to allow the developer to select an inclusive range of re_tests input lines while retaining its original TAP numbering. This makes the triggering cases practical to stress independently. The re_tests lines 2221 through 2222 also pass with 100,000 iterations, including under ASAN with a refcounted stack (PERL_RC_STACK).
demerphq
added a commit
that referenced
this pull request
Sep 14, 2026
Fix intermittent segfaults from the trie logic, as reported in #24816, and discussed in #24814. The list compiler previously recorded a slot after a sparse frame in lasttrans without always allocating or clearing it. A lookup could therefore treat uninitialized transition data as belonging to its state and use an arbitrary next state. This issue affected all platforms but was most prominently visible on Windows and Cygwin. (Historically Windows realloc is not as good as *nix realloc.) As part of fixing this, I reworked some of the logic to be more robust and ensure we never see this kind of issue again. We give the root state the complete octet frame at slots 1 through 256. Other states can reuse unoccupied root slots. Each state base maps octet zero to an offset not less than 1, and we ensure there are enough valid null transitions at the end, so we never read a bogus transition and do not need to do bounds checks either. While working on this I noticed we had some logic left over from the old implementation that still used the concept of "charid", which confused matters. This was removed, and we now have simpler runtime logic to do transition lookups. I also added code to validate the state and transition indexes under DEBUG_r. Move that block outside DEBUG_r when validation is wanted in every full DEBUGGING build. Thus, compilation under C<use re 'debug'> also exercises the transition validation, so if you suspect a related bug try the pattern under that pragma. Added support to regexp.t to allow the developer to select an inclusive range of re_tests input lines while retaining its original TAP numbering. This makes the triggering cases practical to stress independently. The re_tests lines 2221 through 2222 also pass with 100,000 iterations, including under ASAN with a refcounted stack (PERL_RC_STACK).
demerphq
added a commit
that referenced
this pull request
Sep 14, 2026
Fix intermittent segfaults from the trie logic, as reported in #24816, and discussed in #24814. The list compiler previously recorded a slot after a sparse frame in lasttrans without always allocating or clearing it. A lookup could therefore treat uninitialized transition data as belonging to its state and use an arbitrary next state. This issue affected all platforms but was most prominently visible on Windows and Cygwin. (Historically Windows realloc is not as good as *nix realloc.) As part of fixing this, I reworked some of the logic to be more robust and ensure we never see this kind of issue again. We give the root state the complete octet frame at slots 1 through 256. Other states can reuse unoccupied root slots. Each state base maps octet zero to an offset not less than 1, and we ensure there are enough valid null transitions at the end, so we never read a bogus transition and do not need to do bounds checks either. While working on this I noticed we had some logic left over from the old implementation that still used the concept of "charid", which confused matters. This was removed, and we now have simpler runtime logic to do transition lookups. I also added code to validate the state and transition indexes under DEBUG_r. Move that block outside DEBUG_r when validation is wanted in every full DEBUGGING build. Thus, compilation under C<use re 'debug'> also exercises the transition validation, so if you suspect a related bug try the pattern under that pragma. Added support to regexp.t to allow the developer to select an inclusive range of re_tests input lines while retaining its original TAP numbering. This makes the triggering cases practical to stress independently. The re_tests lines 2221 through 2222 also pass with 100,000 iterations, including under ASAN with a refcounted stack (PERL_RC_STACK).
demerphq
force-pushed
the
yves/regen_targets_and_optimizations
branch
from
September 14, 2026 10:28
e258d7b to
a326385
Compare
tonycoz
reviewed
Sep 14, 2026
| cd .. && perl regen\keywords.pl | ||
|
|
||
| regen_invlist regen-invlist : | ||
| cd .. && perl.exe -Ilib regen\mk_invlists.pl |
Contributor
There was a problem hiding this comment.
you don't need the .exe when invoking perl (repeated below)
Collaborator
Author
There was a problem hiding this comment.
I dont mind changing it, but when I reviewed the file it seemed to use this form throughout.
tonycoz
reviewed
Sep 14, 2026
| cd .. && perl regen\keywords.pl | ||
|
|
||
| regen_invlist regen-invlist : | ||
| cd .. && perl.exe -Ilib regen\mk_invlists.pl |
Contributor
There was a problem hiding this comment.
Similarly here for the .exe
Collaborator
Author
There was a problem hiding this comment.
Likewise here, the only case omitting the extension is the line i added for the keywords.
Collaborator
Author
There was a problem hiding this comment.
FWIW, i just went the other way, and changed the regen\keywords case to add the .exe. At least its consistent. If we want to remove the extension in bulk we could do that as a independent commit.
Enhance regen_all and regen-all as the complete regeneration workflow across the supported makefile variants. The workflow builds Perl, runs the standard generators, regenerates keywords and Unicode data, updates distribution metadata, and finishes with a final build. On *nix it includes regen_perly, which is excluded on other platforms because it requires Bison. Add individual targets and diagnostics for the keyword, inversion-list, and character-class generators. Also adds regen_keywords which uses the installed Perl because Devel::Tokenizer::C is not part of the build tree, additionally the generator now reports how to install the missing dependency. Adds and documents REGEN_VERBOSE alongside -v for regeneration scripts and update the porting regeneration test with target-specific recovery commands.
Reduce the cost of the hot mktables paths by using direct reference addresses, compact Range and Range_List objects, direct gap searches, and small lookup caches. Keep the generated data unchanged apart from the generator updates needed for the optimized path. On the same full Unicode mktables workload, the measured run improved from about 17.0 seconds and 208 MB peak RSS to 13.1 seconds and 173 MB peak RSS. That is about 23 percent less wall time and 17 percent less peak memory. The result varies slightly between runs. For the mk_invlists-related Unicode generation, this full workload is the relevant end-to-end measurement. The MPH squeeze benchmark improved from about 34.2 seconds with randomized squeeze to 9.1 seconds with frequency-weighted two-direction squeeze. The blob grew from 9,132 to 9,212 bytes, an increase of 80 bytes or 0.88 percent. The mk_invlists changes preserve the generated data while avoiding expansion of large ranges during native translation. The generated Unicode and character-class outputs were regenerated with the updated generators and syntax checks pass for the changed Perl programs.
demerphq
force-pushed
the
yves/regen_targets_and_optimizations
branch
from
September 15, 2026 08:05
a326385 to
28e9129
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds make targets for our collected regen tools, and make sure the targets are available on all our build targets.
This greatly enhances the 'regen_all' target, to regen everything, which (sadly) requires two complete makes, but so be it.
Along with this is code to optimize mk_invlists.pl, mph.pl and mktables.pl