-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[HWASan] Compatible with Windows path retrieval #172194
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
Conversation
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (GkvJwa) ChangesFix #134853 Since the Windows path separator is typically Full diff: https://github.com/llvm/llvm-project/pull/172194.diff 1 Files Affected:
diff --git a/compiler-rt/lib/hwasan/scripts/hwasan_symbolize b/compiler-rt/lib/hwasan/scripts/hwasan_symbolize
index 19d948d1f0aaa..efc7c868696c3 100755
--- a/compiler-rt/lib/hwasan/scripts/hwasan_symbolize
+++ b/compiler-rt/lib/hwasan/scripts/hwasan_symbolize
@@ -476,7 +476,7 @@ def main():
# Source location.
paths_to_cut = args.source or []
if not paths_to_cut:
- paths_to_cut.append(os.getcwd() + '/')
+ paths_to_cut.append(os.getcwd().replace('\\', '/') + '/')
if 'ANDROID_BUILD_TOP' in os.environ:
paths_to_cut.append(os.environ['ANDROID_BUILD_TOP'] + '/')
|
| paths_to_cut = args.source or [] | ||
| if not paths_to_cut: | ||
| paths_to_cut.append(os.getcwd() + '/') | ||
| paths_to_cut.append(os.getcwd().replace('\\', '/') + '/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is python3, can we just use pathlib? I think they deal with the platform specific separators in a nice abstracted way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from pathlib import Path
// On Windows, directly calling Path.cwd() also uses \\
path = Path.cwd()
>>> print(path)
C:\Users\a
// Calling as_posix can convert to /
path = Path.cwd().as_posix()
>>> print(path)
C:/Users/a
Requires explicit use of as_posix
The result is acceptable; it can be modified to cwd().as_posix().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that is better. because your original code would theoretically replace a '' if it was part of cwd on unix, which is unlikely but still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, review again
The file header use
#!/usr/bin/env python3
I also removed the Python 2 compatibility logic.
|
Thanks |
Fix #134853
Since the Windows path separator is typically
\\, replace it with/.