Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid the possibility of dangling pointers while processing capture items in pcre2_compile #248

Merged
merged 1 commit into from
May 9, 2023

Conversation

carenas
Copy link
Contributor

@carenas carenas commented May 8, 2023

Raised by gcc 13 (from MinGW and Fedora 38):

src/pcre2_compile.c: In function 'compile_regex':
src/pcre2_compile.c:8319:17: warning: storing the address of local variable 'capitem' in '*cb.open_caps' [-Wdangling-pointer=]
 8319 |   cb->open_caps = &capitem;
      |   ~~~~~~~~~~~~~~^~~~~~~~~~

While cb.open_caps is not needed outside the possibly recursive calls to compile_regex() and compile_branch(), it is safer to not keep it with possibly dangling references once they are out of scope.

@carenas carenas marked this pull request as draft May 8, 2023 08:19
@PhilipHazel
Copy link
Collaborator

I'm missing something here. I don't see how passing the chain of open_capitems via its own argument is any different to passing it as a member of the compile block structure. The items themselves are still on the stack. Oh, perhaps the idea is to confine this value to the nested compile_{branch,regex} functions? OK, I think I can understand that. In which case, the patch looks straightforward.

@carenas carenas marked this pull request as ready for review May 8, 2023 21:32
As shown by the following warning from gcc13:

  src/pcre2_compile.c: In function 'compile_regex':
  src/pcre2_compile.c:8319:17: warning: storing the address of local variable 'capitem' in '*cb.open_caps' [-Wdangling-pointer=]
   8319 |   cb->open_caps = &capitem;
        |   ~~~~~~~~~~~~~~^~~~~~~~~~

While normally the stack of "capitems" will be unwinded and the head of
it that is stored in "cb" set to NULL, this is not done in case of a
compilation failure.

Instead of keeping it inside "cb", initialize it in the stack of
compile_regex() and make sure it is gone once that function returns.
@carenas
Copy link
Contributor Author

carenas commented May 8, 2023

Updated the commit message to hopefully make it clearer.

@PhilipHazel PhilipHazel merged commit cf6f984 into PCRE2Project:master May 9, 2023
@carenas carenas deleted the capitem branch May 9, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants