subject:"\[Lldb\-commits\] \[lldb\] \[lldb\] colorize symbols in image lookup with a regex pattern \(PR #69422\)"

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-12 Thread David Spickett via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 



@@ -246,8 +246,8 @@ class Address {
   /// \see Address::DumpStyle
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
 DumpStyle fallback_style = DumpStyleInvalid,
-uint32_t addr_byte_size = UINT32_MAX,
-bool all_ranges = false) const;
+uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false,
+const char *pattern = nullptr) const;

DavidSpickett wrote:

So in code that would be roughly:
```
struct information {
pattern, prefix, sufix
}

PutCStringHighlighted(., std::optional pattern_info);
```

Which tells the reader clearly that if they want to provide any one of the 3 
they must provide all 3. Otherwise they must provide none of them.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-12 Thread David Spickett via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 

@@ -246,8 +246,8 @@ class Address {
   /// \see Address::DumpStyle
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
 DumpStyle fallback_style = DumpStyleInvalid,
-uint32_t addr_byte_size = UINT32_MAX,
-bool all_ranges = false) const;
+uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false,
+const char *pattern = nullptr) const;

DavidSpickett wrote:

>When we use std::move(regex2) to pass it to test_func3, it's essentially 
>moved, and after the move, the regex2 in LookupSymbolInModule becomes garbage.

This is where I get on my Rust powered soapbox and preach about C++ move not 
being what you'd think it is. :)

Good research anyway. Sound like it's what I suspected is that the regex is 
carrying around the match "engine"'s state.

On the struct, yes, the struct itself would be optional. If you do pass an 
instance of it you must provide all 3.

To be honest, once we're packing all this into a struct, that's quite an 
obvious signal to the programmer that this is no ordinary string. So you could 
do just try the struct idea without using the `Regex` type.

And ok, it's not perfect because all 3 struct members have the same type still, 
but if making the pattern a `Regex` is this much trouble it's probably not 
worth it.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-12 Thread via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 

@@ -246,8 +246,8 @@ class Address {
   /// \see Address::DumpStyle
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
 DumpStyle fallback_style = DumpStyleInvalid,
-uint32_t addr_byte_size = UINT32_MAX,
-bool all_ranges = false) const;
+uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false,
+const char *pattern = nullptr) const;

taalhaataahir0102 wrote:

Hi @DavidSpickett! Sorry for the delay 

`test_func3(strm, std::move(regex2))` works but it gives the following problem. 
When we use `std::move(regex2)` to pass it to `test_func3`, it's essentially 
moved, and after the move, the `regex2` in `LookupSymbolInModule` becomes 
garbage.

So in functions where pattern is moved multiple times like this:

```
static void DumpAddress(ExecutionContextScope *exe_scope,
const Address _addr, bool verbose, bool all_ranges,
Stream , std::optional pattern = 
std::nullopt) {
 ...
 ...
 so_addr.Dump(, exe_scope, Address::DumpStyleResolvedDescription,
   Address::DumpStyleInvalid, UINT32_MAX, false, 
std::move(pattern));
 ...
 ...
 if (verbose) {
strm.EOL();
so_addr.Dump(, exe_scope, Address::DumpStyleDetailedSymbolContext,
 Address::DumpStyleInvalid, UINT32_MAX, all_ranges, 
std::move(pattern));
  }
 ...
}
```
The second `std::move(pattern)`, does not move the original pattern value but 
some garbage value. The only way is to make a copy of the original pattern and 
then use it with `std::move(copy_pattern)` every time (which doesn't look okay 
:-/).

This is the example code and output if I couldn't explain it properly above:
CODE:
```
static void test_func3(Stream , const std::optional pattern = 
std::nullopt) {
  if (pattern.has_value() && pattern.value().match("main")) {
strm.Printf("Pattern matches 'main' inside test_func3\n");
}
}

static uint32_t LookupSymbolInModule(CommandInterpreter ,
 Stream , Module *module,
 const char *name, bool name_is_regex,
 bool verbose, bool all_ranges) {
  // test_func1(strm, std::make_optional(name));
  std::optional regex2 = std::make_optional(name);
  if (regex2.has_value()) {
strm.Printf("Pattern has a value\n");
if(regex2.value().match("main")) {
  strm.Printf("Pattern matches main\n");
}
else{
  strm.Printf("Pattern doesn't match main\n");
}
  } else {
strm.Printf("Pattern doesn't have a value\n");
  }
  test_func3(strm, std::move(regex2));
  if (regex2.has_value()) {
strm.Printf("Pattern has a value\n");
if(regex2.value().match("main")) {
  strm.Printf("Pattern matches main\n");
}
else{
  strm.Printf("Pattern doesn't match main\n");
}
  } else {
strm.Printf("Pattern doesn't have a value\n");
  }  ...
  ...
}
```
OUTPUT:
```
talha@talha-ROG-Strix-G513QE-G513QE:~/Desktop/LLDB/build/bin$ ./lldb a.out 
(lldb) target create "a.out"
Current executable set to '/home/talha/Desktop/LLDB/build/bin/a.out' (x86_64).
(lldb) image lookup -r -s main
Pattern has a value
Pattern matches main
Pattern matches 'main' inside test_func3
Pattern has a value
Pattern doesn't match main
...
...
```
So after `std::move(pattern)`, `pattern != "main"` within the same function 

Similarly passing `std::make_optional(name)` to func3 also causes 
problem problem:
i.e., `test_func3(strm, std::make_optional(name));`

When we directly pass `std::make_optional(name)`, it creates a 
temporary `std::optional` object that is passed directly to 
`test_func3`. This temporary object can be moved without requiring an explicit 
`std::move` because I guess temporaries are considered rvalues, and 
`std::optional` has appropriate move constructors to handle them.
But storing the result of `std::make_optional(name)` in a variable 
and then passing it without using `std::move`, if the variable is passed as an 
argument to the function, it attempts to perform a copy unless we use 
`std::move`. So, initially we can pass it directly to the next dumping function 
but further passing it will require `std::move` and the same problem comes 
back. 

Also 1 last thing for clarification, as discussed later i.e., 

> I think we may be able to reduce the boilerplate by putting the 
> prefix/suffix/pattern in a struct, and passing that as an optional assuming 
> we can figure out the move issue with regex. But this is for a future change 
> if at all.

The struct will be optional and not the

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-12 Thread via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


https://github.com/taalhaataahir0102 edited 
https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-08 Thread via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


taalhaataahir0102 wrote:

Sure will try to make it work for windows. Hoping it will be as easier to build 
lldb on windows as it's for linux 

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-08 Thread David Spickett via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


DavidSpickett wrote:

Spoke too soon, had to disable the test on Windows: 
https://github.com/llvm/llvm-project/commit/810d09faf89af53025205c540ef9980e2286e687

Making it work there may require making function lookup work, not sure.

And Mac OS: 
https://github.com/llvm/llvm-project/commit/61f18255fab3c404dc43a59091a750c22e5d0ccb

Which I think is a matter of finding the right regex for the CHECK. If either 
of you have access to Windows/Mac, please give it a go.

Otherwise, @JDevlieghere could you (/one of your colleagues) try the test on 
Mac and see what can be done?

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-08 Thread David Spickett via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior
Message-ID:
In-Reply-To:

DavidSpickett wrote:

Ok bots look good, so congratulations on landing this change! :partying_face:

(you will have some email from build bots, it's a good idea to go look at the
failures just to see what they look like, but I have fixed them)

Sounds like you already have ideas for follow ups. You should also add a
release note
(https://github.com/llvm/llvm-project/blob/main/llvm/docs/ReleaseNotes.rst#changes-to-lldb)
at some point when you're happy with the feature overall. Or in other words,
when it can be described in 1 or 2 sentences without a lot of caveats.

I'm trying the feature myself and noticing the thing you said about the symbol
context. Even when looking up symbols on a running process, when we print a
`Summary: ` line, `target_sp` is always nullptr. Surprising, and a bit annoying
as a majority of the symbols in my searches didn't have a `Name: ` line.

(though one can work around this by adding `-v`)

Could be that there's never been a need for target here, and as you said, what
you really want is the debugger settings. So perhaps the better route is to add
a link back to those.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

67 matches

Mail list logo