Skip to content

[Backtracing][Linux] Replace regex with ProcMapsScanner. #66449

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 9, 2023
Merged

[Backtracing][Linux] Replace regex with ProcMapsScanner. #66449

merged 1 commit into from
Jun 9, 2023

Conversation

@al45tair
Copy link
Contributor

@al45tair al45tair commented Jun 8, 2023 •
edited by AnthonyLatsis
Loading

We were using a regular expression to scan /proc/<pid>/maps, but we shouldn't really do that because it breaks non-bootstrapped builds.

rdar://110452324
Resolves #66457

@al45tair al45tair requested review from gottesmm and mikeash June 8, 2023 13:55
@al45tair
Copy link
Contributor Author

al45tair commented Jun 8, 2023

@swift-ci Please smoke test

mikeash
mikeash approved these changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

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

This is an excellent example of the power of regex.

stdlib/public/Backtracing/ProcMapsScanner.swift Outdated
Copy link
Contributor

@mikeash mikeash Jun 8, 2023

Choose a reason for hiding this comment

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

Using UInt8(ascii:) here would save people from looking up what ASCII 9 is, and be more consistent with the rest.

Copy link
Contributor Author

@al45tair al45tair Jun 8, 2023

Choose a reason for hiding this comment

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

:-) I'm probably showing my age there; I'll change to using UInt8(ascii:) for all of them.

@al45tair
[Backtracing][Linux] Replace regex with ProcMapsScanner.
d396656 
We were using a regular expression to scan `/proc/<pid>/maps`, but we
shouldn't really do that because it breaks non-bootstrapped builds.

rdar://110452324
@al45tair al45tair force-pushed the eng/PR-110452324 branch from 466e733 to d396656 Compare June 8, 2023 14:53
@al45tair
Copy link
Contributor Author

al45tair commented Jun 8, 2023

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

al45tair commented Jun 8, 2023

This is an excellent example of the power of regex.

+1 on that. There is a reason I wanted to use a regex here. I suspect the state machine is faster, but probably not by nearly enough to justify the extra time to write it.

@al45tair
Copy link
Contributor Author

al45tair commented Jun 8, 2023

Using UInt8(ascii:) here would save people from looking up what ASCII 9 is

What? You mean someone doesn't know what ASCII 9 is? :-D

I sometimes forget I grew up in a world of 8-bit micros and dot matrix printers, so ended up just knowing lots of ASCII control codes.

@al45tair al45tair mentioned this pull request Jun 9, 2023
Closed
@al45tair al45tair merged commit e6ca3f0 into swiftlang:main Jun 9, 2023
@al45tair al45tair mentioned this pull request Jun 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikeash mikeash mikeash approved these changes

@gottesmm gottesmm gottesmm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Building stdlib/public/Backtracing/Backtrace.swift fails

3 participants

@al45tair @mikeash @gottesmm