Sanchda/rust demangling#53
Conversation
| #pragma once | ||
| #include <string> | ||
|
|
||
| namespace RustDemangler { |
There was a problem hiding this comment.
Oh, namespace :)
First time I see it here. It's been a long time, I almost forgot you could do this in C++ 🤣
There was a problem hiding this comment.
Heh, mostly here I was afraid of conflicting with something else in the project (I'm not familiar with the codebase). Happy to remove if it would align with the design of the repo. 🙏
There was a problem hiding this comment.
I don't think it is necessary to remove it - I assume it is a purely syntactic construct and won't result in linking more of libstdc++ to the final library ...
| const char *end = ptr + str.size() - hash_pre.size() - hash_eg.size(); | ||
| for (; ptr <= end; ++ptr) { | ||
| if (*ptr == '$') { | ||
| if (ptr[1] == '$') { |
There was a problem hiding this comment.
I guess this is safe because the string has the tailing hash and there never will be $ as the last char here.
There was a problem hiding this comment.
Indeed, and also because end excludes the hash part, so we're guaranteed to have several characters of "string" left after any ptr.
| } else { | ||
| // We didn't have valid unicode values, but we should still skip | ||
| // the $u??$ sequence | ||
| ret += str.substr(i, k_nb_read_chars); |
There was a problem hiding this comment.
Is this skipping the $u??$ sequence or copying it to the ret string?
There was a problem hiding this comment.
Good question. It "skips" the $u???$ pattern in the sense that it doesn't perform additional processing on it, but it actually inserts the unprocessed $u???$ sequence into the destination string. In other words, tokens are only consumed when they are successfully matched, otherwise they are inserted literally into the result.
There was a problem hiding this comment.
Can you extend the comment with 'inserted, not processed' part? Thanks!
jbachorik
left a comment
jbachorik
left a comment
There was a problem hiding this comment.
Hi, I have a bunch of noob questions. Please, be gentle :)
Better comment
|
Looks like none of the code quality nags were related to this PR. Merged |
2 participants
What does this PR do?:
Adds legacy Rust demangling to native code
Motivation:
I saw a flamegraph with tokio in the native parts and thought this might be useful.
Additional Notes:
Rust demangling isn't free, but I haven't really spent much time trying to speed it up. This was taken from a similar implementation in the native profiler, but adapted to C++11
How to test the change?:
I added tests. I believe this is fairly comprehensive, as it pulls from both llvm and libiberty.
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.