Skip to content

pcre2: Update to upstream version 10.42 (take two) - #70472

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
akien-mga:pcre2-10.42
Jun 12, 2023
Merged

pcre2: Update to upstream version 10.42 (take two)#70472
akien-mga merged 1 commit into
godotengine:masterfrom
akien-mga:pcre2-10.42

Conversation

@akien-mga

@akien-mga akien-mga commented Dec 23, 2022

Copy link
Copy Markdown
Member

Changelog: https://github.com/PCRE2Project/pcre2/blob/pcre2-10.42/ChangeLog

This fixes support for RISC-V architectures in the sljit library, so we enable the module's compilation for rv64 too.

Redo of #70447 which I reverted due to a regression described there:
#70447 (comment)

Fixes #76174


From @vonagam:

This PR caused those lines to appear (repeated multiple times) in unit tests logs at the start.

ERROR: Condition "p_ptr == nullptr" is true.
   at: free_static (core/os/memory.cpp:148)

While tests do not fail because of that, this does not seem right...
Or is it unavoidable known issue?

My early findings:

Backtrace:

(gdb) bt
#0  Memory::free_static (p_ptr=0x0, p_pad_align=false) at core/os/memory.cpp:149
#1  0x0000000006c247fb in _regex_free (ptr=0x0, user=0x0) at modules/regex/regex.cpp:43
#2  0x0000000006c1cb84 in pcre2_match_32 (code=0xc9019b0, subject=0xc8a74c0, length=9, start_offset=0, options=0, match_data=0xc830060, mcontext=0xc9863b0) at thirdparty/pcre2/src/pcre2_match.c:6846
#3  0x0000000006c25c85 in RegEx::search (this=0x7fffffffc300, p_subject=..., p_offset=0, p_end=-1) at modules/regex/regex.cpp:219
#4  0x00000000057cfee7 in TestRegEx::DOCTEST_ANON_FUNC_6556 () at ./modules/regex/tests/test_regex.h:81
#5  0x00000000053d282d in doctest::Context::run (this=0x7fffffffd0e0) at ./thirdparty/doctest/doctest.h:6930
#6  0x00000000057d5540 in test_main (argc=2, argv=0x7fffffffd728) at tests/test_main.cpp:174
#7  0x0000000005393ec2 in Main::test_entrypoint (argc=2, argv=0x7fffffffd728, tests_need_run=@0x7fffffffd1bf: true) at main/main.cpp:608
#8  0x0000000005333cc2 in main (argc=2, argv=0x7fffffffd728) at platform/linuxbsd/godot_linuxbsd.cpp:55

It probably comes from this change:

9. Removed the use of an initial backtracking frames vector on the system stack
in pcre2_match() so that it now always uses the heap. (In a multi-thread
environment with very small stacks there had been an issue.) This also is
tidier for JIT matching, which didn't need that vector. The heap vector is now
remembered in the match data block and re-used if that block itself is re-used.
It is freed with the match data block.

We likely need to change our _regex_free (and maybe _regex_malloc) to take this new heap vector into account somehow.

@akien-mga akien-mga added enhancement topic:thirdparty cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Dec 23, 2022
@akien-mga akien-mga added this to the 4.0 milestone Dec 23, 2022
@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 10, 2023
@lmurray lmurray mentioned this pull request Jun 11, 2023
@akien-mga akien-mga added needs work and removed cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Jun 12, 2023
Changelog: https://github.com/PCRE2Project/pcre2/blob/pcre2-10.42/ChangeLog

This fixes support for RISC-V architectures in the sljit library, so we
enable the module's compilation for `rv64` too.
@akien-mga

Copy link
Copy Markdown
Member Author

Added a nullptr check in _regex_free as suggested by @lawnjelly, and it solved the error spam.

I don't know if there are other implications for that upstream change which caused this error spam in the first place.

@akien-mga

Copy link
Copy Markdown
Member Author

For 4.0 and 3.5, I'm making a simpler PR that just adds the null check, to fix #76174: #78137.

@RedworkDE

Copy link
Copy Markdown
Member

While the pcre2 docs don't appear to specify anything about the required behavior of the passed free function, the standard C free function is explicitly specified to do nothing when passed a null pointer, so I would assume this is just caused by some simplifications and them removing some otherwise unnecessary null checks.

@akien-mga
akien-mga merged commit 0b43728 into godotengine:master Jun 12, 2023
@akien-mga
akien-mga deleted the pcre2-10.42 branch June 12, 2023 15:32
@akien-mga

Copy link
Copy Markdown
Member Author

Cherry-picked for 3.6.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 29, 2023
Comment on lines 153 to 156
#if MAC_OS_X_VERSION_MIN_REQUIRED >= 110000
pthread_jit_write_protect_np(enable_exec);
#elif defined(__clang__)
if (__builtin_available(macOS 11.0, *))
pthread_jit_write_protect_np(enable_exec);
#else
#error "Must target Big Sur or newer"

@aaronfranke aaronfranke Sep 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of this PR cherry-picked into the 3.x branch, I am unable to compile Godot on macOS, because Godot 3.x targets macOS 10.15+, and this change breaks support for macOS < 11.0.

If I revert this PR, it compiles fine (fails on linking but that's a different problem).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to compile Godot on macOS, because Godot 3.x targets macOS 10.15+, and this change breaks support for macOS < 11.0.

Target version should be changed to 11.0, 10.15 for arm64 existed only as the early alphas.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in #82007

BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
pcre2: Update to upstream version 10.42 (take two)
BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
pcre2: Update to upstream version 10.42 (take two)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RegEx.search_all causes 'Condition "p_ptr == nullptr" is true.' error with PCRE2 10.42 (Linux distro packages)

4 participants