[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef
zturner added a subscriber: tatyana-krasnukha. zturner added a comment. It would be nice if You could replace the logic that iterates these arrays. We no longer need to terminate on a sentinel nullptr entry and can now use range based for loop Repository: rLLDB LLDB https://reviews.llvm.org/D52572 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef
zturner added a comment. You might be right, I’m on mobile so hard for me to review. I saw a bunch of nullptr so assumed it was still using sentinel values for iterating. If not, ignore my suggestion Repository: rLLDB LLDB https://reviews.llvm.org/D52572 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef
zturner added a comment. No, separate revision is fine. Thanks! Repository: rLLDB LLDB https://reviews.llvm.org/D52572 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a subscriber: aleksandr.urakov. zturner added a comment. It requires a lot of work (nobody has started porting it). lldb-server exists on other platforms but it basically needs a full port to Windows. It doesn’t use the same process plugin, but it would instead use a different plugin that would copy much of the low level code. Once it works well enough the existing process plugin would then be deleted. It would be great if someone had time to work on it, but I understand if for practical reasons other work has higher priority https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52468: [PDB] Treat `char`, `signed char` and `unsigned char` as three different types
zturner accepted this revision. zturner added a comment. Can you change the description of the patch before submitting it? It's hard to understand why the change does what the description says it does, because the description mentions 3 types of chars but the patch only handles 1. I would just make the description say "handle char as a builtin type" or something Repository: rLLDB LLDB https://reviews.llvm.org/D52468 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52627: [lldb] Start a new line for the next output if there are no symbols in the current symtab
zturner added a comment. Couple of options: 1. Can you give an example of before/after output? 2. Is the `size_t` change related? 3. Can you use -U99 in the future when generating patches? Repository: rLLDB LLDB https://reviews.llvm.org/D52627 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52627: [lldb] Start a new line for the next output if there are no symbols in the current symtab
zturner added a comment. s/options/comments/ Repository: rLLDB LLDB https://reviews.llvm.org/D52627 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52626: [lldb] Remove an assertion in RichManglingContext::GetBufferRef() hit when debugging a native x86 Windows process
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. BTW, I wrote a demangler for Windows ABI that should be 100% complete modulo bugs and can generate an AST that RichManglingContext should use. So it would be interesting if someone decided to start using that to make this actually work. Repository: rLLDB LLDB https://reviews.llvm.org/D52626 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. I think it’s fine. Eventually when you are ready to support remote debugging hopefully we can convert it over to using lldb-server https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. One idea would be to define some lit substitutions like %debuginfo. It’s true you can produce a gcc style command line that will be equivalent to a clang-cl invocation but it won’t be easy. eg you’ll needing to pass -fms-compatibility as well as various -I for includes. It may be easier to have substitutions instead https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. In https://reviews.llvm.org/D52618#1251076, @stella.stamenova wrote: > In https://reviews.llvm.org/D52618#1250909, @zturner wrote: > > > One idea would be to define some lit substitutions like %debuginfo. It’s > > true you can produce a gcc style command line that will be equivalent to a > > clang-cl invocation but it won’t be easy. eg you’ll needing to pass > > -fms-compatibility as well as various -I for includes. > > > > It may be easier to have substitutions instead > > > Another option would be to define a way in lit to specify a command to run > based on requirements - similar how we can use "windows" or "linux" in the > "requires" command. Yea that would work too. `REQUIRES` isn't quite the right thing because that just makes the infrastructure decide whether to run or skip your test. It would need to be something different, like `COMPILATION_SETTINGS: debug, opt, noexcept`. But I think that would be quite a bit of work and probably not fit nicely with the existing `ShTest`. You might need a subclass of `ShTest` like `LLDBShTest` that can extend its functionality a bit. https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. In https://reviews.llvm.org/D52618#1252372, @labath wrote: > In https://reviews.llvm.org/D52618#1250909, @zturner wrote: > > > One idea would be to define some lit substitutions like %debuginfo. It’s > > true you can produce a gcc style command line that will be equivalent to a > > clang-cl invocation but it won’t be easy. eg you’ll needing to pass > > -fms-compatibility as well as various -I for includes. > > > > It may be easier to have substitutions instead > > > Using substitutions SGTM. > > I am not sure if this is a good idea, but it had occured to me that we could > put `-fms-compatibility` and friends into a substitution of it's own, which > would be computed by lit (through some equivalent of `clang++ -###` ?). That > way, the tests could still use g++ syntax, only the command lines would > contain an extra `%cflags` argument. This has the advantage of extra > flexibility over a predefined set of compile commands (%compile_with_debug, > %compile_without_debug, ...), and it might be sufficient to make > cross-compiling work, if we ever need it. Another idea I just thought of, which would basically be the heaviest hammer possible and give the most flexibility is to modify the lit infrastructure (just for lldb, not all of llvm) to look for a `compiler_config.py` file. If it finds it, it can run the file. This file could define whatever substitutions it wanted. In the top-level LLDB lit configuration, we could provide some basic common infrastructure to easily support common use cases. I don't have specifics in mind for how the implementation would look like, but from a user point of view (i.e. what the `compiler_config.py` would look like), I'm imagining you could write something like this: # compiler_config.py global compiler_config compiler_config.create_configuration( "fooconfig", # user can reference this config as 'fooconfig' from a test file. driver=best,# use clang-cl on Windows, clang++ otherwise debug=True, # pass /Z7 on Windows, -g otherwise optlevel=1, # pass /O2 on Windows, -O2 otherwise output_type=sharedlib, # pass -fPIC on Linux and /D_DLL on Windows exceptions=False, # pass -fno-exceptions on Linux, /EHs-c- on Windows rtti=False, # pass -fno-rtti on Linux, /GR- on Windows mode=compile-only) # Don't run the linker compiler_config.create_configuration( "barconfig", # user can reference this config as 'barconfig' from a test file. driver=g++, # this config always invokes clang++, never anything else. debug=False, optlevel=3, output_type=exe) # etc Then, in your test file, you could have: ; RUN: %fooconfig %p/foo.cpp ; RUN: %barconfig %p/bar.cpp This is a very rough outline of the idea, but I think this could actually be really cool if done properly. https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857
zturner added a comment. > By the way, what do you think, how can we make LLDB support aligned stacks? > As far as I know, similar alignment problems are reproducible on non-Windows > too. When you see VFRAME, you need to look in FPO data. As you might have guessed, VFRAME only occurs in X86. I compiled your sample program with a few modifications, and I'll show some output of `llvm-pdbutil` to illustrate how analyzing FPO data works (which would also give you some insight into how this will eventually be implemented without DIA) Here is the source code I compiled: int g_var = ; void __fastcall with_double(short arg_0, float arg_1) { char loc_0 = 'x'; double dvar = 0.5678; } void __fastcall with_float(short arg_0, float arg_1) { char loc_0 = 'x'; float fvar = 0.5678f; } int main(int argc, char *argv[]) { bool loc_0 = true; int loc_1 = ; with_double(, 0.1234); with_float(, 0.1234); return 0; } Then I ran this command. $ llvm-pdbutil.exe dump -symbols -modi=0 vlt.pdb Symbols Mod | `D:\src\llvmbuild\cl\Debug\x64\vlt.obj`: 52 | S_GPROC32 [size = 52] `with_double` parent = 0, end = 268, addr = 0001:, code size = 50 type = `0x1001 (void (short, float))`, debug start = 0, debug end = 0, flags = none 104 | S_FRAMEPROC [size = 32] size = 28, padding size = 0, offset to padding = 0 bytes of callee saved registers = 0, exception handler addr = : local fp reg = VFRAME, param fp reg = EBP flags = 236 | S_LOCAL [size = 16] `dvar` type=0x0041 (double), flags = none 252 | S_DEFRANGE_FRAMEPOINTER_REL [size = 16] offset = -16, range = [0001:0027,+23) gaps = 2 268 | S_END [size = 4] 272 | S_GPROC32 [size = 52] `with_float` parent = 0, end = 484, addr = 0001:0064, code size = 44 type = `0x1001 (void (short, float))`, debug start = 0, debug end = 0, flags = none 324 | S_FRAMEPROC [size = 32] size = 16, padding size = 0, offset to padding = 0 bytes of callee saved registers = 0, exception handler addr = : local fp reg = EBP, param fp reg = EBP flags = 452 | S_LOCAL [size = 16] `fvar` type=0x0040 (float), flags = none 468 | S_DEFRANGE_FRAMEPOINTER_REL [size = 16] offset = -8, range = [0001:0087,+21) gaps = 2 484 | S_END [size = 4] the `S_GPROC32` and `S_END` records form a pair, so all relevant data for this function is inside of the matching pair. Both `dvar` and `fvar` are of type `S_DEFRANGE_FRAMEPOINTER_REL`, which means they're relative to the framepointer. So we need to search for the `S_FRAMEPROC` record inside of this function. It's immediately after the `S_GPROC32` record in both cases. In the case of `with_float` we find that it says "local fp reg = EBP". This means it's easy, nothing special to do which is why it fixed the issue for you changing to float. On the other hand, as you noticed the other one says `VFRAME`. This means we need to go to the FPO data. But first we need to find the address of this function. The `S_GPROC32` of `with_double` says it's at address `0001:`. So we check the section headers to find out what is section 1. $ llvm-pdbutil.exe dump -section-headers vlt.pdb Section Headers SECTION HEADER #1 .text name 3E3FD virtual size 1000 virtual address 3E400 size of raw data 600 file pointer to raw data 0 file pointer to relocation table 0 file pointer to line numbers 0 number of relocations 0 number of line numbers 6020 flags IMAGE_SCN_CNT_CODE IMAGE_SCN_MEM_EXECUTE IMAGE_SCN_MEM_READ So now we know section 1 starts at virtual address `0x1000`, and this particular function is at offset ``, so it is also at virtual address `0x1000`. The function has size 50, so we are looking for an FPO record in the range of `[0x1000,0x1032)` Now let's look at the FPO data in the PDB. Old FPO Data RVA| Code | Locals | Params | Prolog | Saved Regs | Use BP | Has SEH | Frame Type 131A | 20 | 0 | 0 | 0 | 0 | false | false | FPO 1483 | 19 | 0 | 0 | 0 | 0 | false | false | FPO New FPO Data RVA| Code | Locals | Params | Stack | Prolog | Saved Regs | Has
[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857
zturner added a comment. You didn't include it here, but I notice the test file just writes `clang-cl /Zi`. Should we be passing `-m32` or `-m64`? Currently, the test just runs with whatever architecture happens to be set via the VS command prompt. The behavior here is different on x86 and x64 so perhaps it requires separate tests. Repository: rLLDB LLDB https://reviews.llvm.org/D53086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53090: [ProcessWindows] Fix a bug that causes lldb to freeze
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Nice find, thanks Repository: rLLDB LLDB https://reviews.llvm.org/D53090 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()
zturner added a comment. If you plan to invest in more substantial changes in `ObjectFilePECOFF`, it might worth considering a complete re-write in terms of `llvm::object::coff`. It has pretty comprehensive support for the PE/COFF spec, so it's just a matter of implementing `ObjectFilePECOFF` on top of it. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:810 + + std::lock_guard guard(module_sp->GetMutex()); + Does this actually need to be a `recursive_mutex`? Repository: rLLDB LLDB https://reviews.llvm.org/D53094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53166: [lldbsuite] Fix the filecheck functionality to work with Python 3
zturner added inline comments. Comment at: packages/Python/lldbsuite/test/lldbtest.py:2230-2233 +# In Python 2, communicate sends byte strings. In Python 3, communicate sends bytes. +# If we got a string (and not a byte string), encode it before sending. +if isinstance(output, str) and not isinstance(output, bytes): +output = output.encode() Ok I had to stare at this for a long time to figure out what was going on. I think you need to update the comment here, because it makes it sounds like `output` is the result of a `Popen.communicate`. But `output` is the result of a `debugger.HandleCommand()`. I think we should actually just leave this the way it was and fix it in the call to `Popen` (see below) Comment at: packages/Python/lldbsuite/test/lldbtest.py:2247-2249 subproc = Popen(filecheck_args, stdin=PIPE, stdout=PIPE, stderr=PIPE) cmd_stdout, cmd_stderr = subproc.communicate(input=output) cmd_status = subproc.returncode If I'm not mistaken, Python 3 can accept `stdin` as `bytes` or `string`, and either will work, it depends on the value of `universal_newlines`, and `stdout` will be in whatever format `stdin` was in. If we pass `universal_newlines=True`, then it will expect a `string` and output a `string`, otherwise it expects a `bytes` and output a `bytes`. Given that `FileCheck` is supposed to operate on textual data and not binary data, perhaps we should be doing that here? Repository: rLLDB LLDB https://reviews.llvm.org/D53166 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
zturner added a subscriber: vsk. zturner added a comment. See the other email thread. The bots have been broken since September, all of them complain about missing FileCheck executable Repository: rLLDB LLDB https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
zturner added a subscriber: aprantl. zturner added a comment. https://reviews.llvm.org/D53002 Is the thread I'm referring to. Repository: rLLDB LLDB https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error
zturner added a subscriber: vsk. zturner added a comment. Why is FileCheck missing in the first place? https://reviews.llvm.org/D53175 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857
zturner added a comment. In https://reviews.llvm.org/D53086#1261697, @aleksandr.urakov wrote: > Thanks a lot for so detailed answer, it helps! > > So we need to parse a FPO program and to convert it in a DWARF expression > too. The problem here (in the DIA case) is that I don't know how to retrieve > the required FPO range (we have a symbol context when creating a variable, > but it seems that it doesn't contain enough information). I think the `SymbolContext` should have enough information. As long as you can find the `PDBSymbolFunction` for the current frame, that gives you the range of the function, and the `IDiaSymbol` for the variable should give you the important info like register, offset, etc. > As for the raw PDB case, can the same `S_LOCAL` have several > `S_DEFRANGE_FRAMEPOINTER_REL` records for several ranges? If so, then the > problem exists for the raw PDB case too, but there's a solution: we can > combine processing of all `S_DEFRANGE_FRAMEPOINTER_REL` records in the single > DWARF expression (and call the required FPO program depending on current > `eip`). Not as far as I know. There should be only 1. But note that is not the only type of record that can appear, there are several others. But basically there will be `S_LOCAL` followed by 1 record describing the location. > The interesting moment here is that MSVC doesn't emit correct information for > locals. For the program `aaa.cpp`: > > void bar(char a, short b, int c) { } > > void __fastcall foo(short arg_0, float arg_1) { > char loc_0 = 'x'; > double __declspec(align(8)) loc_1 = 0.5678; > bar(1, 2, 3); > } > > int main(int argc, char *argv[]) { > foo(, 0.1234); > return 0; > } > > > compiled with `cl /Zi /Oy aaa.cpp` we have the next disassembly of `foo`: > > pushebp > mov ebp, esp > and esp, 0FFF8h > sub esp, 10h > mov [esp+4], cx ; arg_0 > mov byte ptr [esp+3], 'x' ; loc_0 > movsd xmm0, ds:__real@3fe22b6ae7d566cf > movsd qword ptr [esp+8], xmm0 ; loc_1 > push3 ; c > push2 ; b > push1 ; a > callj_?bar@@YAXDFH@Z ; bar(char,short,int) > add esp, 0Ch > mov esp, ebp > pop ebp > retn4 > > > but MSVC emits the next info about locals: > > 296 | S_GPROC32 [size = 44] `foo` > parent = 0, end = 452, addr = 0001:23616, code size = 53 > type = `0x1003 (void (short, float))`, debug start = 14, debug end = > 47, flags = has fp > 340 | S_FRAMEPROC [size = 32] > size = 16, padding size = 0, offset to padding = 0 > bytes of callee saved registers = 0, exception handler addr = > : > local fp reg = VFRAME, param fp reg = EBP > flags = has async eh | opt speed > 372 | S_REGREL32 [size = 20] `arg_0` > type = 0x0011 (short), register = ESP, offset = 4294967284 > 392 | S_REGREL32 [size = 20] `arg_1` > type = 0x0040 (float), register = EBP, offset = 8 > 412 | S_REGREL32 [size = 20] `loc_1` > type = 0x0041 (double), register = ESP, offset = 4294967288 > 432 | S_REGREL32 [size = 20] `loc_0` > type = 0x0070 (char), register = ESP, offset = 4294967283 > 452 | S_END [size = 4] > > > so neither LLDB nor Visual Studio show valid values (except for `arg_1`). Generally speaking, if you're using `/Oy` all bets are off and good luck :) Interestingly, clang actually gets this right but we use a totally different CodeView record. $ llvm-pdbutil.exe dump -symbols -modi=0 foo-clang.pdb | grep -A 4 loc_1 392 | S_LOCAL [size = 16] `loc_1` type=0x0041 (double), flags = none 408 | S_DEFRANGE_FRAMEPOINTER_REL [size = 16] offset = -16, range = [0001:0075,+51) gaps = 2 D:\src\llvmbuild\ninja-release-x64> If you try to debug the same program in VS built with clang with the exact same command line, it will work. I need to study the disassembly and FPO program of the cl.exe version some more to decide if the bug is in the compiler or the debugger, but I think the bug is in the compiler. It's funny because our optimized debug info is not very good, so I'm surprised there's a case where we're better than MSVC here :) https://reviews.llvm.org/D53086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857
zturner added a comment. In https://reviews.llvm.org/D53086#1263001, @zturner wrote: > In https://reviews.llvm.org/D53086#1261697, @aleksandr.urakov wrote: > > > Thanks a lot for so detailed answer, it helps! > > > > So we need to parse a FPO program and to convert it in a DWARF expression > > too. The problem here (in the DIA case) is that I don't know how to > > retrieve the required FPO range (we have a symbol context when creating a > > variable, but it seems that it doesn't contain enough information). > > > I think the `SymbolContext` should have enough information. As long as you > can find the `PDBSymbolFunction` for the current frame, that gives you the > range of the function, and the `IDiaSymbol` for the variable should give you > the important info like register, offset, etc. Do we have access to the current instruction pointer? That's what you need to find the correct FPO record. https://reviews.llvm.org/D53086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52461: [PDB] Introduce `PDBNameParser`
zturner added a comment. In https://reviews.llvm.org/D52461#1265335, @aleksandr.urakov wrote: > Hello! > > I just have tried to patch `CPlusPlusNameParser` in the way to support MSVC > demangled names, but there is a problem. `CPlusPlusNameParser` splits an > incoming name in tokens with `clang::Lexer`. I've lexed the next name: > > `anonymous namespace'::foo > > > The lexer treats the first character (a grave accent) as an unknown token, > and it's ok for our purposes. Then it sees an identifier (`anonymous`), a > keyword (`namespace`), and it's ok too. But the problem is with the last part > of the string. The lexer sees an apostrophe and supposes that it's a > character constant, it looks for a closing apostrophe, don't find it and > treats all the line ending (`'::foo`) as a single unknown token. > > It is possible to somehow make `clang::Lexer` lex MSVC demangled names > correctly, but I'm not sure if it is the right place to do it. And it may > have then some side effects during lexing a real code. > > Another option is to somehow preprocess the name before lexing and replace > all //paired// apostrophes with grave accents, and after lexing replace with > apostrophes back, and make `CPlusPlusNameParser` understand unknown grave > accent tokens. But it's a bit tricky, may be you can suggest some better > solution? Just handle the `anonymous namespace' thing specially before passing to `CPlusPlusNameParser`. Repository: rLLDB LLDB https://reviews.llvm.org/D52461 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53402: [SymbolFileNativePDB] Fix missing linkage to DebugInfoCodeView
zturner added a subscriber: mgorny. zturner added a comment. Lgtm Repository: rLLDB LLDB https://reviews.llvm.org/D53402 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
zturner added a comment. > PDB has no enough info to restore VBase offsets properly, so we have to read > real VTable instead. What's missing that you're unable to restore the VBase offset properly? Repository: rLLDB LLDB https://reviews.llvm.org/D53506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
zturner added a comment. In https://reviews.llvm.org/D53506#1270919, @aleksandr.urakov wrote: > In https://reviews.llvm.org/D53506#1270893, @zturner wrote: > > > What's missing that you're unable to restore the VBase offset properly? > > > If I understand correctly, in the PDB there is only info about offset to > VTablePtr and index in VTable, so there is enough info to retrieve VBase > offset fairly, and we do it in that way. But there's no info in PDB about > offset to VBase directly from object. This info is used when the "fair" > doesn't work (e.g. at line 6640). This patch just makes the "fair" way to > work in more cases. My understanding of record layout with virtual bases is still sketchy (it's very confusing), and it's even worse with DIA because the API is so general and poorly documented, so let's go to the low-level CodeView records. typedef struct lfVBClass { unsigned short leaf; // LF_VBCLASS (virtual base) | LV_IVBCLASS (indirect virtual base) CV_fldattr_tattr; // attribute CV_typ_tindex; // type index of direct virtual base class CV_typ_tvbptr; // type index of virtual base pointer unsigned char vbpoff[CV_ZEROLEN]; // virtual base pointer offset from address point // followed by virtual base offset from vbtable } lfVBClass; This is what we have access to reading directly from the raw pdb file, which is sometimes more information than what we have access to using DIA. Of course, we also have to interpret whether this actually means what we think it means by inspecting the bytes of a C++ object in a debugger and comparing the layout to what the debug info tells us. So, the point is, just because we don't have access to the info via DIA doesn't mean we won't have access to the info once the native pdb plugin is complete. Just something to think about. Repository: rLLDB LLDB https://reviews.llvm.org/D53506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
zturner created this revision. zturner added reviewers: labath, lemo, aleksandr.urakov, stella.stamenova, asmith. Herald added subscribers: arphaman, mgorny. This is the minimal set of functionality necessary to support type lookup by name in the native PDB plugin. For the purposes of testing I decided to bypass `lldb-test` and go with a scripted lldbinit file. I'm sticking with this approach wherever possible to prove that this is "true" cross-platform debugger functionality. However, there are definite limitations. For example, the output format of `type lookup` has some limitations. It doesn't print the layout of the type in any kind of detail (e.g. field offsets)`, it doesn't support lookup by regex, it doesn't print the underlying type of an enumeration, it doesn't support limiting the scope of the search to specific kinds of types, so there is definitely room for `lldb-test` to come into the picture here to expand the coverage since we have full control over the output format. I tried to think of some interesting test cases to exercise some edge cases here, but I welcome some ideas for other interesting cases. I consciously avoided things like bit fields, member functions, pointers to members, and virtual bases because there was a balancing act between implementing a useful piece of functionality and keeping the patch small enough that it can be understandable enough to review meaningfully. Welcome any feedback. Hopefully this is getting to the point that maybe it's hackable by others as well, although I'm definitely going to continue working on it. https://reviews.llvm.org/D53511 Files: lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit lldb/lit/SymbolFile/NativePDB/tag-types.cpp lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt lldb/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h === --- /dev/null +++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h @@ -0,0 +1,68 @@ +//===-- SymbolFileNativePDB.h ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H +#define LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H + +#include "lldb/Symbol/ClangASTImporter.h" +#include "llvm/DebugInfo/CodeView/CVRecord.h" +#include "llvm/DebugInfo/CodeView/TypeRecord.h" +#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h" + +#include "PdbSymUid.h" + +namespace clang { +class CXXBaseSpecifier; +class TagDecl; +} // namespace clang + +namespace lldb_private { +class Type; +class CompilerType; +namespace npdb { +class SymbolFileNativePDB; + +class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { + union UdtTagRecord { +UdtTagRecord() {} +llvm::codeview::UnionRecord ur; +llvm::codeview::ClassRecord cr; +llvm::codeview::EnumRecord er; + } m_cvr; + + PdbSymUid m_uid; + CompilerType &m_derived_ct; + clang::TagDecl &m_tag_decl; + SymbolFileNativePDB &m_symbol_file; + std::vector m_bases; + ClangASTImporter::LayoutInfo m_layout; + +public: + UdtRecordCompleter(PdbSymUid uid, CompilerType &derived_ct, + clang::TagDecl &tag_decl, + SymbolFileNativePDB &symbol_file); + +#define MEMBER_RECORD(EnumName, EnumVal, Name) \ + llvm::Error visitKnownMember(llvm::codeview::CVMemberRecord &CVR,\ + llvm::codeview::Name##Record &Record) override; +#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName) +#include "llvm/DebugInfo/CodeView/CodeViewTypes.def" + + void complete(); + +private: + lldb::opaque_compiler_type_t + AddBaseClassForTypeIndex(llvm::codeview::TypeIndex ti, + llvm::codeview::MemberAccess access); +}; + +} // namespace npdb +} // namespace lldb_private + +#endif // LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp === --- /dev/null +++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -0,0 +1,188 @@ +#include "UdtRecordCompleter.h" + +#include "PdbIndex.h" +#include "PdbSymUid.h" +#include "PdbUtil.h" +#include "SymbolFileNativePDB.h" + +#include "lldb/Symbol/ClangASTContext.h" +#include "lldb/Symbol
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
zturner added a subscriber: aleksandr.urakov. zturner added a comment. To answer your question, PE/COFF executable symbol tables are basically empty Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
zturner added inline comments. Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:87 +// Test single inheritance. +class Derived : public Class { +public: lemo wrote: > - at least one virtual function + override? > - at least one method returning void? The reason I didn't add any tests for methods is because I didn't add anything in the implementation to parse method records either. As I said in the patch description, it was a balance between providing some piece of useful functionality while keeping the patch size down. So I tried to limit the scope to just fields minus bitfields, since there's some extra complexity there that I wanted to punt on for now. The reason I have the constructor is because it was required in order to initialize the reference member, otherwise it wouldn't compile. Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:110 + +// Test multiple inheritance, as well as protected and private inheritance. +class Derived2 : protected Class, private Struct { lemo wrote: > just an idea - add a virtual inheritance variation? Virtual inheritance is a good followup, but if you look at `UdtRecordCompleter.cpp` and Ctrl+F for `VirtualBaseClassRecord`, I basically treat them as regular base classes and I put a fixme to handle the virtual aspects of the base. So that's a known problem right now. Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:131 + +int main(int argc, char **argv) { + Struct S; lemo wrote: > a few suggestions for additional things to cover: > - local classes > - lambdas > - instantiating class and function templates >- explicit specializations >- for classes, partial specializations > - namespaces > - anonymous namespace Some of these I could probably handle right now. I tried to keep the scope of the patch to "types which would be named", because it makes it easy to write tests (the test can just do lookup by name, basically a WinDbg `dt` test.). That makes local classes, lambdas, anonymous namespace, and anything to do with functions out of scope. https://reviews.llvm.org/D53511 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB345047: [NativePDB] Add basic support for tag types to the native pdb plugin. (authored by zturner, committed by ). Herald added subscribers: teemperor, abidh. Changed prior to commit: https://reviews.llvm.org/D53511?vs=170447&id=170683#toc Repository: rLLDB LLDB https://reviews.llvm.org/D53511 Files: lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit lit/SymbolFile/NativePDB/tag-types.cpp source/Plugins/SymbolFile/NativePDB/CMakeLists.txt source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -10,6 +10,7 @@ #ifndef LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_SYMBOLFILENATIVEPDB_H #define LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_SYMBOLFILENATIVEPDB_H +#include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/SymbolFile.h" #include "llvm/ADT/DenseMap.h" @@ -144,22 +145,63 @@ llvm::pdb::PDBFile &GetPDBFile() { return m_index->pdb(); } const llvm::pdb::PDBFile &GetPDBFile() const { return m_index->pdb(); } + ClangASTContext &GetASTContext() { return *m_clang; } + ClangASTImporter &GetASTImporter() { return *m_importer; } + private: + void AddBaseClassesToLayout(CompilerType &derived_ct, + ClangASTImporter::LayoutInfo &layout, + const llvm::codeview::ClassRecord &record); + void AddMembersToLayout(ClangASTImporter::LayoutInfo &layout, + const llvm::codeview::TagRecord &record); + void AddMethodsToLayout(ClangASTImporter::LayoutInfo &layout, + const llvm::codeview::TagRecord &record); + + size_t FindTypesByName(llvm::StringRef name, uint32_t max_matches, + TypeMap &types); + + lldb::TypeSP CreateModifierType(PdbSymUid type_uid, + const llvm::codeview::ModifierRecord &mr); + lldb::TypeSP CreatePointerType(PdbSymUid type_uid, + const llvm::codeview::PointerRecord &pr); + lldb::TypeSP CreateSimpleType(llvm::codeview::TypeIndex ti); + lldb::TypeSP CreateTagType(PdbSymUid type_uid, + const llvm::codeview::ClassRecord &cr); + lldb::TypeSP CreateTagType(PdbSymUid type_uid, + const llvm::codeview::EnumRecord &er); + lldb::TypeSP CreateTagType(PdbSymUid type_uid, + const llvm::codeview::UnionRecord &ur); + lldb::TypeSP + CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, + clang::TagTypeKind ttk, + clang::MSInheritanceAttr::Spelling inheritance); + lldb::FunctionSP GetOrCreateFunction(PdbSymUid func_uid, const SymbolContext &sc); lldb::CompUnitSP GetOrCreateCompileUnit(const CompilandIndexItem &cci); + lldb::TypeSP GetOrCreateType(PdbSymUid type_uid); + lldb::TypeSP GetOrCreateType(llvm::codeview::TypeIndex ti); lldb::FunctionSP CreateFunction(PdbSymUid func_uid, const SymbolContext &sc); lldb::CompUnitSP CreateCompileUnit(const CompilandIndexItem &cci); + lldb::TypeSP CreateType(PdbSymUid type_uid); + lldb::TypeSP CreateAndCacheType(PdbSymUid type_uid); llvm::BumpPtrAllocator m_allocator; lldb::addr_t m_obj_load_address = 0; std::unique_ptr m_index; + std::unique_ptr m_importer; + ClangASTContext *m_clang = nullptr; + + llvm::DenseMap m_decl_to_status; + + llvm::DenseMap m_uid_to_decl; llvm::DenseMap m_functions; llvm::DenseMap m_compilands; + llvm::DenseMap m_types; }; } // namespace npdb Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp === --- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -0,0 +1,188 @@ +#include "UdtRecordCompleter.h" + +#include "PdbIndex.h" +#include "PdbSymUid.h" +#include "PdbUtil.h" +#include "SymbolFileNativePDB.h" + +#include "lldb/Symbol/ClangASTContext.h" +#include "lldb/Symbol/Type.h" +#include "lldb/Utility/LLDBAssert.h" +#include "lldb/lldb-enumerations.h" +#include "lldb/lldb-forward.h" + +#include "llvm/DebugInfo/CodeView/TypeDeserializer.h" +#include "llvm/DebugInfo/CodeView/TypeIndex.h" +#include "llvm/DebugInfo/PDB/Native/TpiStream.h" +#include "llvm/DebugInfo/PDB/PDBTypes.h" + +using namespace llvm::codeview; +using namespace llvm::pdb; +using namespace lldb; +using nam
[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y
zturner created this revision. zturner added reviewers: labath, davide, clayborg. Herald added a subscriber: JDevlieghere. [NFC] Refactor SetBaseClasses and DeleteBaseClasses. We currently had a 2-step process where we had to call SetBaseClassesForType and DeleteBaseClasses. Every single caller followed this exact 2-step process, and there was manual memory management going on with raw pointers. We can do better than this by storing a vector of unique_ptrs and passing this around. This makes for a cleaner API, and we only need to call one method so there is no possibility of a user forgetting to call DeleteBaseClassSpecifiers. In addition to this, it also makes for a *simpler* API. Part of why I wanted to do this is because when I was implementing the native PDB interface I had to spend some time understanding exactly what I was deleting and why. ClangAST has significant mental overhead associated with it, and reducing the API surface can go along way to making it simpler for people to understand. https://reviews.llvm.org/D53590 Files: lldb/include/lldb/Symbol/ClangASTContext.h lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Symbol/ClangASTContext.cpp lldb/unittests/Symbol/TestClangASTContext.cpp Index: lldb/unittests/Symbol/TestClangASTContext.cpp === --- lldb/unittests/Symbol/TestClangASTContext.cpp +++ lldb/unittests/Symbol/TestClangASTContext.cpp @@ -337,15 +337,19 @@ EXPECT_NE(nullptr, non_empty_base_field_decl); EXPECT_TRUE(ClangASTContext::RecordHasFields(non_empty_base_decl)); + std::vector> bases; + // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived); - CXXBaseSpecifier *non_empty_base_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, false, false); - bool result = m_ast->SetBaseClassesForClassType( - empty_derived.GetOpaqueQualType(), &non_empty_base_spec, 1); + std::unique_ptr non_empty_base_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, false, false); + bases.push_back(std::move(non_empty_base_spec)); + bool result = m_ast->TransferBaseClasses(empty_derived.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_base_cxx_decl = @@ -363,10 +367,12 @@ nullptr, lldb::eAccessPublic, "EmptyDerived2", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived2); - CXXBaseSpecifier *non_empty_vbase_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, true, false); - result = m_ast->SetBaseClassesForClassType(empty_derived2.GetOpaqueQualType(), - &non_empty_vbase_spec, 1); + std::unique_ptr non_empty_vbase_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, true, false); + bases.push_back(std::move(non_empty_vbase_spec)); + result = m_ast->TransferBaseClasses(empty_derived2.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived2); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_vbase_cxx_decl = @@ -377,9 +383,6 @@ empty_derived_non_empty_vbase_cxx_decl, false)); EXPECT_TRUE( ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl)); - - delete non_empty_base_spec; - delete non_empty_vbase_spec; } TEST_F(TestClangASTContext, TemplateArguments) { Index: lldb/source/Symbol/ClangASTContext.cpp === --- lldb/source/Symbol/ClangASTContext.cpp +++ lldb/source/Symbol/ClangASTContext.cpp @@ -8239,39 +8239,37 @@ #pragma mark C++ Base Classes -clang::CXXBaseSpecifier * +std::unique_ptr ClangASTContext::CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type, AccessType access, bool is_virtual, bool base_of_class) { - if (type) -return new clang::CXXBaseSpecifier(
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner created this revision. zturner added reviewers: clayborg, jingham, labath. Herald added subscribers: atanasyan, JDevlieghere, sdardis. When we get the `resolve_scope` parameter from the SB API, it's a `uint32_t`. We then pass it through all of LLDB this way, as a uint32. This is unfortunate, because it means the user of an API never actually knows what they're dealing with. We can call it something like `resolve_scope` and have comments saying "this is a value from the `SymbolContextItem` enumeration, but it makes more sense to just have it actually *be* the correct type in the actual C++ type system to begin with. This way the person reading the code just knows what it is. There is one thing here which I'm a little uncertain about, which is that I included a file from llvm `llvm/ADT/BitmaskEnum.h` from `lldb/lldb-enumerations.h`, which is a public header.` This is only done for convenience, and has two effects. 1. It allows us to use bitwise operations on enums so we don't have to write things like `Foo x = Foo(eFoo1 | eFoo2)` 2. More subtly, it inserts an enumerator into the enum. But (!) it doesn't change the value of any existing enumerators and it does so with a name that won't cause any clashes, so the important thing is that it shouldn't cause any name clashes. Putting this all together, I think this should be an ABI-compatible change as far as SWIG is concerned, and I can't see any differences on my end. But if anyone can see any reason why we should be wary of this, speak up. Assuming all goes well with this patch, I plan to repeat the same thing with `SymbolFile::GetTypes` which currently has a `uint32_t types_mask` https://reviews.llvm.org/D53597 Files: lldb/include/lldb/Core/Address.h lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/include/lldb/Symbol/CompileUnit.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/include/lldb/Target/StackFrame.h lldb/include/lldb/lldb-enumerations.h lldb/source/API/SBAddress.cpp lldb/source/API/SBFrame.cpp lldb/source/API/SBModule.cpp lldb/source/API/SBTarget.cpp lldb/source/Commands/CommandObjectSource.cpp lldb/source/Core/Address.cpp lldb/source/Core/Disassembler.cpp lldb/source/Core/Module.cpp lldb/source/Core/ModuleList.cpp lldb/source/Core/SourceManager.cpp lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/source/Symbol/CompileUnit.cpp lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/source/Target/StackFrame.cpp Index: lldb/source/Target/StackFrame.cpp === --- lldb/source/Target/StackFrame.cpp +++ lldb/source/Target/StackFrame.cpp @@ -35,6 +35,8 @@ #include "lldb/Target/Thread.h" #include "lldb/Utility/RegisterValue.h" +#include "lldb/lldb-enumerations.h" + using namespace lldb; using namespace lldb_private; @@ -262,7 +264,8 @@ // StackFrame object, everyone will have as much information as possible and no // one will ever have to look things up manually. //-- -const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { +const SymbolContext & +StackFrame::GetSymbolContext(SymbolContextItem resolve_scope) { std::lock_guard guard(m_mutex); // Copy our internal symbol context into "sc". if ((m_flags.Get() & resolve_scope) != resolve_scope) { @@ -314,7 +317,7 @@ // haven't already tried to lookup one of those things. If we haven't // then we will do the query. - uint32_t actual_resolve_scope = 0; + SymbolContextItem actual_resolve_scope = SymbolContextItem(0); if (resolve_scope & eSymbolContextCompUnit) { if (m_flags.IsClear(eSymbolContextCompUnit)) { Index: lldb/source/Symbol/SymbolVendor.cpp === --- lldb/source/Symbol/SymbolVendor.cpp +++ lldb/source/Symbol/SymbolVendor.cpp @@ -235,7 +235,7 @@ } uint32_t SymbolVendor::Res
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1273087, @jingham wrote: > It would be great not to have to use comments to express what these values > mean. OTOH, having to put in static casts whenever you want to or values > together would be a pain, so if there's no way to do it without magic, I'm a > little less enthused... > > But on the magic: It looks like BitMaskEnum.h pulls in MathTypes.h which > pulls in Compiler.h which then pulls in llvm-config.h. You are supposed to > be able to build tools on top of lldb with just the headers that go in > LLDB.framework, you are not required to have the full source tree for an lldb > build. I don't want to impose that restriction without very good reason, and > fixing this wart doesn't rise to that level. Anyway, so if we want to > include BitMaskEnum.h we would be required to ship a bunch of llvm headers > (including some build produced ones IIUC). I don't think that's a very good > idea. > > It also looks to me like the value of the largest item + 1 gets baked into > the operator overloading code. What would happen if we decided to add an > element to the enum? It's not the largest item +1, it's actually just that the largest item gets a second internal name. If you add a new element to the enum you need to just make sure you update the `LLVM_MARK_AS_BITMASK_ENUM()` to contain the new largest value. Anyway, your point about `llvm-config.h` is a good one, so what I can perhaps do instead is make something called `LLDB_DEFINE_BITMASK_ENUM(EnumName)` which basically just defines the overloads for a particular enum, then we can have it be a two step process. 1. Make the enum, 2. Call `LLDB_DEFINE_BITMASK_ENUM(Enum)`. This way there's no external header dependencies. I'll upload a new patch shortly. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y
zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3273-3274 + is_virtual, is_base_of_class); + if (!result) +break; + If the result is a `nullptr`, then there is no base to begin with. This indicates an error, so continuing doesn't make sense. In fact, I think this was a bug with the previous implementation. We were pushing back a null base onto the list of base classes. I suspect it never actually happened in practice, though. https://reviews.llvm.org/D53590 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y
zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3273-3274 + is_virtual, is_base_of_class); + if (!result) +break; + zturner wrote: > If the result is a `nullptr`, then there is no base to begin with. This > indicates an error, so continuing doesn't make sense. In fact, I think this > was a bug with the previous implementation. We were pushing back a null base > onto the list of base classes. I suspect it never actually happened in > practice, though. One alternative would be to assert that it is *not* null, then continue as the previous code did. WDYT? https://reviews.llvm.org/D53590 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner updated this revision to Diff 170737. zturner added a comment. Remove the reference to `llvm/ADT/BitmaskEnu.h` and define the operators ourselves. Also, removed a few casts that got left in. https://reviews.llvm.org/D53597 Files: lldb/include/lldb/Core/Address.h lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/include/lldb/Symbol/CompileUnit.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/include/lldb/Target/StackFrame.h lldb/include/lldb/lldb-enumerations.h lldb/source/API/SBAddress.cpp lldb/source/API/SBFrame.cpp lldb/source/API/SBModule.cpp lldb/source/API/SBTarget.cpp lldb/source/Commands/CommandObjectSource.cpp lldb/source/Core/Address.cpp lldb/source/Core/Disassembler.cpp lldb/source/Core/Module.cpp lldb/source/Core/ModuleList.cpp lldb/source/Core/SourceManager.cpp lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/source/Symbol/CompileUnit.cpp lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/source/Target/StackFrame.cpp Index: lldb/source/Target/StackFrame.cpp === --- lldb/source/Target/StackFrame.cpp +++ lldb/source/Target/StackFrame.cpp @@ -35,6 +35,8 @@ #include "lldb/Target/Thread.h" #include "lldb/Utility/RegisterValue.h" +#include "lldb/lldb-enumerations.h" + using namespace lldb; using namespace lldb_private; @@ -262,7 +264,8 @@ // StackFrame object, everyone will have as much information as possible and no // one will ever have to look things up manually. //-- -const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { +const SymbolContext & +StackFrame::GetSymbolContext(SymbolContextItem resolve_scope) { std::lock_guard guard(m_mutex); // Copy our internal symbol context into "sc". if ((m_flags.Get() & resolve_scope) != resolve_scope) { @@ -314,7 +317,7 @@ // haven't already tried to lookup one of those things. If we haven't // then we will do the query. - uint32_t actual_resolve_scope = 0; + SymbolContextItem actual_resolve_scope = SymbolContextItem(0); if (resolve_scope & eSymbolContextCompUnit) { if (m_flags.IsClear(eSymbolContextCompUnit)) { Index: lldb/source/Symbol/SymbolVendor.cpp === --- lldb/source/Symbol/SymbolVendor.cpp +++ lldb/source/Symbol/SymbolVendor.cpp @@ -235,7 +235,7 @@ } uint32_t SymbolVendor::ResolveSymbolContext(const Address &so_addr, -uint32_t resolve_scope, +SymbolContextItem resolve_scope, SymbolContext &sc) { ModuleSP module_sp(GetModule()); if (module_sp) { @@ -248,7 +248,7 @@ uint32_t SymbolVendor::ResolveSymbolContext(const FileSpec &file_spec, uint32_t line, bool check_inlines, -uint32_t resolve_scope, +SymbolContextItem resolve_scope, SymbolContextList &sc_list) { ModuleSP module_sp(GetModule()); if (module_sp) { Index: lldb/source/Symbol/SymbolFile.cpp === --- lldb/source/Symbol/SymbolFile.cpp +++ lldb/source/Symbol/SymbolFile.cpp @@ -97,7 +97,7 @@ uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec, uint32_t line, bool check_inlines, - uint32_t resolve_scope, + lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list) { return 0; } Index: lldb/source/Symbol/CompileUnit.cpp === --- lldb/source/Symbol/Co
[Lldb-commits] [PATCH] D53616: Don't type-erase the FunctionNameType or TypeClass enums
zturner created this revision. zturner added reviewers: davide, jingham, clayborg. Herald added a subscriber: JDevlieghere. This is a followup to https://reviews.llvm.org/D53597, with 2 more enums. Assuming that patch is good, I see no reason why this wasn't isn't good as well, but I'm throwing it out here for review anyway if nothing else as an FYI. I'll wait for a final LGTM on https://reviews.llvm.org/D53597, but assuming that's good, I'll probably submit both of these unless someone has a comment or comments on this one first. I think this is the last of the type-erased enums, so after this, all of our flags enums should be strongly typed through the system. https://reviews.llvm.org/D53616 Files: lldb/include/lldb/Breakpoint/BreakpointResolverName.h lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/include/lldb/Target/Target.h lldb/include/lldb/lldb-enumerations.h lldb/source/API/SBCompileUnit.cpp lldb/source/API/SBModule.cpp lldb/source/API/SBTarget.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Commands/CommandObjectBreakpoint.cpp lldb/source/Core/Module.cpp lldb/source/Core/ModuleList.cpp lldb/source/Expression/IRExecutionUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/source/Target/Target.cpp Index: lldb/source/Target/Target.cpp === --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -415,12 +415,11 @@ false); } -BreakpointSP -Target::CreateBreakpoint(const FileSpecList *containingModules, - const FileSpecList *containingSourceFiles, - const char *func_name, uint32_t func_name_type_mask, - LanguageType language, lldb::addr_t offset, - LazyBool skip_prologue, bool internal, bool hardware) { +BreakpointSP Target::CreateBreakpoint( +const FileSpecList *containingModules, +const FileSpecList *containingSourceFiles, const char *func_name, +FunctionNameType func_name_type_mask, LanguageType language, +lldb::addr_t offset, LazyBool skip_prologue, bool internal, bool hardware) { BreakpointSP bp_sp; if (func_name) { SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList( @@ -443,9 +442,9 @@ Target::CreateBreakpoint(const FileSpecList *containingModules, const FileSpecList *containingSourceFiles, const std::vector &func_names, - uint32_t func_name_type_mask, LanguageType language, - lldb::addr_t offset, LazyBool skip_prologue, - bool internal, bool hardware) { + FunctionNameType func_name_type_mask, + LanguageType language, lldb::addr_t offset, + LazyBool skip_prologue, bool internal, bool hardware) { BreakpointSP bp_sp; size_t num_names = func_names.size(); if (num_names > 0) { @@ -465,11 +464,13 @@ return bp_sp; } -BreakpointSP Target::CreateBreakpoint( -const FileSpecList *containingModules, -const FileSpecList *containingSourceFiles, const char *func_names[], -size_t num_names, uint32_t func_name_type_mask, LanguageType language, -lldb::addr_t offset, LazyBool skip_prologue, bool internal, bool hardware) { +BreakpointSP +Target::CreateBreakpoint(const FileSpecList *containingModules, + const FileSpecList *containingSourceFiles, + const char *func_names[], size_t num_names, + FunctionNameType func_name_type_mask, + LanguageType language, lldb::addr_t offset, + LazyBool skip_prologue, bool internal, bool hardware) { BreakpointSP bp_sp; if (num_names > 0) { SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList( Index: lldb/source/Symbol/SymbolVendor.cpp === --- lldb/source/Symbol/SymbolVendor.cpp +++ lldb/source/Symbol/SymbolVendor.cpp @@ -288,7 +288,7 @@ size_t SymbolVendor::FindFunctions(const ConstString &name, c
[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()
zturner added a comment. In https://reviews.llvm.org/D53094#1273556, @asmith wrote: > I think this addresses all the previous comments. Still didn't get a clear answer if the mutex being used needs to be recursive. If it doesn't, perhaps `std::mutex` can be used instead of `std::recursive_mutex`? Not everyone agrees with me, but I often prefer to be too strict rather than too relaxed. https://reviews.llvm.org/D53094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner created this revision. zturner added reviewers: clayborg, labath, jingham. Herald added subscribers: JDevlieghere, aprantl. Previously the Module would parse a type name into scope and base name during a lookup and only give the SymbolFile plugin the base name, then it would filter the results down into the match set based on whether it was supposed to be an exact match. However, if this is supposed to be an exact match, a SymbolFile can do this lookup in O(1) time by using its internal accelerator tables, but it can't do this unless it actually receives the full name as well as the information that it is supposed to be an exact match. Rather than being too smart, we should just let the symbol file plugin be the arbiter to decide how it wants to do lookups, so we should pass it the full name. I'm trying to keep this as NFC for the DWARF plugin, so I took the code that was in Module.cpp and tried to rewrite it to be equivalent at the top of `SymbolFileDWARF::FindTypes` https://reviews.llvm.org/D53662 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit lldb/lit/SymbolFile/NativePDB/tag-types.cpp lldb/source/Core/Module.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/tools/lldb-test/lldb-test.cpp Index: lldb/tools/lldb-test/lldb-test.cpp === --- lldb/tools/lldb-test/lldb-test.cpp +++ lldb/tools/lldb-test/lldb-test.cpp @@ -456,8 +456,8 @@ SymbolContext SC; DenseSet SearchedFiles; TypeMap Map; - Vendor.FindTypes(SC, ConstString(Name), ContextPtr, true, UINT32_MAX, - SearchedFiles, Map); + Vendor.FindTypes(SC, Name, ContextPtr, false, true, UINT32_MAX, SearchedFiles, + Map); outs() << formatv("Found {0} types:\n", Map.GetSize()); StreamString Stream; Index: lldb/source/Symbol/SymbolVendor.cpp === --- lldb/source/Symbol/SymbolVendor.cpp +++ lldb/source/Symbol/SymbolVendor.cpp @@ -315,17 +315,18 @@ } size_t SymbolVendor::FindTypes( -const SymbolContext &sc, const ConstString &name, -const CompilerDeclContext *parent_decl_ctx, bool append, size_t max_matches, +const SymbolContext &sc, llvm::StringRef name, +const CompilerDeclContext *parent_decl_ctx, bool exact_match, bool append, +size_t max_matches, llvm::DenseSet &searched_symbol_files, TypeMap &types) { ModuleSP module_sp(GetModule()); if (module_sp) { std::lock_guard guard(module_sp->GetMutex()); if (m_sym_file_ap.get()) - return m_sym_file_ap->FindTypes(sc, name, parent_decl_ctx, append, - max_matches, searched_symbol_files, - types); + return m_sym_file_ap->FindTypes(sc, name, parent_decl_ctx, exact_match, + append, max_matches, + searched_symbol_files, types); } if (!append) types.Clear(); Index: lldb/source/Symbol/SymbolFile.cpp === --- lldb/source/Symbol/SymbolFile.cpp +++ lldb/source/Symbol/SymbolFile.cpp @@ -140,8 +140,8 @@ } uint32_t SymbolFile::FindTypes( -const SymbolContext &sc, const ConstString &name, -const CompilerDeclContext *parent_decl_ctx, bool append, +const SymbolContext &sc, llvm::StringRef name, +const CompilerDeclContext *parent_decl_ctx, bool exact_match, bool append, uint32_t max_matches, llvm::DenseSet &searched_symbol_files, TypeMap &types) { Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -134,10 +134,9 @@ std::vector &mangled_names) override; uint32_t - FindTypes(const lldb_private::SymbolContext &sc, -const lldb_private::ConstString &name, + FindTypes(const lldb_private::SymbolContext &sc, llvm::StringRef name, const lldb_private::CompilerDeclContext *parent_decl_ctx, -bool append, uint32_t max_matches, +bool exact_match, bool append, uint32_t max_matches, llvm::DenseSet &searched_symbol_files,
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. Note that AFAICT, native PDB plugin is now the only plugin where you can do `type lookup -- NS::Struct::Local` and have it return instantly. On the regular PDB plugin it doesn't work at all (returns no results). On the DWARF plugin, I haven't tested, but it will either not work at all, or take a potentially long time if you have a lot of debug info). https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. Hi Greg, any thoughts on this? https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1275238, @jingham wrote: > I worry that your patch changes the behavior when you add the type_class > explicitly in the lookup (i.e. look up "struct Struct" not "Struct". That > should work... > > Note, this doesn't currently work in type lookup: > > (lldb) type lookup "struct Foo" > no type was found matching '"struct Foo"' > Ahh, nice catch. Luckily it should be easy to add a couple of tests for those alongside the other ones I added, so I'll try that and upload a new version. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
zturner added a subscriber: JDevlieghere. zturner added a comment. It seems like FileSpec was moved out of Utility and into Host. I’m not a fan of this change, because it took a lot of effort to get it into Utility, which helped break a lot of bad dependencies. Can we invert this dependency and move FileSpec back into Utility? https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner updated this revision to Diff 171134. zturner added a comment. Fixed issues pointed out by @jingham and added some test coverage for this. https://reviews.llvm.org/D53662 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/include/lldb/Symbol/Type.h lldb/include/lldb/Symbol/TypeMap.h lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit lldb/lit/SymbolFile/NativePDB/tag-types.cpp lldb/source/Core/Module.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeList.cpp lldb/source/Symbol/TypeMap.cpp lldb/tools/lldb-test/lldb-test.cpp lldb/unittests/Symbol/TestType.cpp Index: lldb/unittests/Symbol/TestType.cpp === --- lldb/unittests/Symbol/TestType.cpp +++ lldb/unittests/Symbol/TestType.cpp @@ -21,9 +21,7 @@ const char *expected_scope, const char *expected_name) { llvm::StringRef scope, name; - lldb::TypeClass type_class; - bool is_scoped = - Type::GetTypeScopeAndBasename(full_type, scope, name, type_class); + bool is_scoped = Type::GetTypeScopeAndBasename(full_type, scope, name); EXPECT_EQ(is_scoped, expected_is_scoped); if (expected_is_scoped) { EXPECT_EQ(scope, expected_scope); Index: lldb/tools/lldb-test/lldb-test.cpp === --- lldb/tools/lldb-test/lldb-test.cpp +++ lldb/tools/lldb-test/lldb-test.cpp @@ -456,8 +456,8 @@ SymbolContext SC; DenseSet SearchedFiles; TypeMap Map; - Vendor.FindTypes(SC, ConstString(Name), ContextPtr, true, UINT32_MAX, - SearchedFiles, Map); + Vendor.FindTypes(SC, Name, ContextPtr, false, true, UINT32_MAX, SearchedFiles, + Map); outs() << formatv("Found {0} types:\n", Map.GetSize()); StreamString Stream; Index: lldb/source/Symbol/TypeMap.cpp === --- lldb/source/Symbol/TypeMap.cpp +++ lldb/source/Symbol/TypeMap.cpp @@ -153,18 +153,35 @@ void TypeMap::RemoveMismatchedTypes(const char *qualified_typename, bool exact_match) { + llvm::StringRef name(qualified_typename); llvm::StringRef type_scope; llvm::StringRef type_basename; - TypeClass type_class = eTypeClassAny; + TypeClass type_class = Type::ConsumeTypeClass(name); if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope, - type_basename, type_class)) { + type_basename)) { type_basename = qualified_typename; type_scope = ""; } return RemoveMismatchedTypes(type_scope, type_basename, type_class, exact_match); } +void TypeMap::OnlyKeepTypeClass(lldb::TypeClass type_class) { + if (type_class == eTypeClassAny) +return; + + collection matching_types; + + iterator pos, end = m_types.end(); + + for (const auto &t : m_types) { +TypeClass tc = t.second->GetForwardCompilerType().GetTypeClass(); +if (tc & type_class) + matching_types.insert(t); + } + m_types.swap(matching_types); +} + void TypeMap::RemoveMismatchedTypes(const std::string &type_scope, const std::string &type_basename, TypeClass type_class, bool exact_match) { @@ -193,8 +210,7 @@ llvm::StringRef match_type_scope; llvm::StringRef match_type_basename; if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope, -match_type_basename, -match_type_class)) { +match_type_basename)) { if (match_type_basename == type_basename) { const size_t type_scope_size = type_scope.size(); const size_t match_type_scope_size = match_type_scope.size(); Index: lldb/source/Symbol/TypeList.cpp === --- lldb/source/Symbol/TypeList.cpp +++ lldb/source/Symbol/TypeList.cpp @@ -112,9 +112,10 @@ bool exact_match) { llvm::StringRef type_scope; llvm::StringRef type_basename; - TypeClass type_class = eTypeClassAny; + llvm::StringRef name(qualified_typ
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276086, @clayborg wrote: > As long as Swig is happy and the ABI doesn't change I am ok with this. Will > we see the variables better when debugging? Or is this solely so the > SymbolContextItem type doesn't disappear from the debug info? We will see variables better when debugging too. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276110, @zturner wrote: > In https://reviews.llvm.org/D53597#1276086, @clayborg wrote: > > > As long as Swig is happy and the ABI doesn't change I am ok with this. Will > > we see the variables better when debugging? Or is this solely so the > > SymbolContextItem type doesn't disappear from the debug info? > > > We will see variables better when debugging too. (That was actually my primary motivation) https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276145, @jingham wrote: > So far as I can tell, this patch will make lookup of exact types faster for > PDB, but because of the way DWARF debug_names tables are constructed, I don't > think there's any way we can do the same thing for DWARF. > > But unless I'm misunderstanding the patch, this doesn't change correctness of > the lookup (except for fixing "type lookup 'struct Foo'"). Did I miss > something? > > Jim That's the other patch. This patch is NFC and just makes debugging nicer because you can see enum values in your debugger as rich enumerators. But for the other patch, if what you said is correct, then I suppose that's correct. I asked Eric Christopher and he said he thought (but wasn't 100% sure) that types were hashed in the accelerator tables by their full name and not their base name. If that is true then it could make exact matches faster. But if it's incorrect then yes, it wouldn't be able to make exact matches faster in DWARF. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a subscriber: jingham. zturner added a comment. I guess the question is, How is that hash and the bucket computed? If it's based on the full name, then you should be able to get fast exact lookup. If it's based on the based name, then it will indeed be slow. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
zturner added a comment. I envision `FileSpec` as being more of a `PathSpec`. It should be able manipulate paths as strings and nothing more. There is indeed some logic that still remains that resolves paths, but it manages to do this using LLVM's file system APIs and the only reason it's still there is because it was baked in and a bit hard to remove. One idea for removing it would be to have `FileSpec FileSystem::Resolve(const FileSpec &)`. Then instead of saying `FileSpec foo(path, /*resolve = */ true);`, they could say `FileSpec foo = FileSystem::Resolve(FileSpec(path));` or something. https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
zturner added a comment. In https://reviews.llvm.org/D53532#1276463, @JDevlieghere wrote: > Thanks for the feedback! I totally agree it's a good solution and it was one > of the things I considered. It didn't make it to the top of the list because > it is very intrusive and changes the semantics of FileSpec quite a bit (i.e. > it becomes a PathSpec as Zachary noted). Anyway, I'm happy to do this and > I'll split this up in two patches as Pavel suggested. I'll extend the > FileSystem first, so that we can do the VFS stuff straight away instead of > implementing these methods twice. It does change them, but that's always been the sort of "end goal" of `FileSpec` anyway, it just hasn't been reached yet. Hopefully we can get there with your help :) https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB345312: [NFC] Refactor SetBaseClasses and DeleteBaseClasses. (authored by zturner, committed by ). Changed prior to commit: https://reviews.llvm.org/D53590?vs=170700&id=171185#toc Repository: rLLDB LLDB https://reviews.llvm.org/D53590 Files: include/lldb/Symbol/ClangASTContext.h source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h source/Plugins/SymbolFile/PDB/PDBASTParser.cpp source/Symbol/ClangASTContext.cpp unittests/Symbol/TestClangASTContext.cpp Index: unittests/Symbol/TestClangASTContext.cpp === --- unittests/Symbol/TestClangASTContext.cpp +++ unittests/Symbol/TestClangASTContext.cpp @@ -337,15 +337,19 @@ EXPECT_NE(nullptr, non_empty_base_field_decl); EXPECT_TRUE(ClangASTContext::RecordHasFields(non_empty_base_decl)); + std::vector> bases; + // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived); - CXXBaseSpecifier *non_empty_base_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, false, false); - bool result = m_ast->SetBaseClassesForClassType( - empty_derived.GetOpaqueQualType(), &non_empty_base_spec, 1); + std::unique_ptr non_empty_base_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, false, false); + bases.push_back(std::move(non_empty_base_spec)); + bool result = m_ast->TransferBaseClasses(empty_derived.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_base_cxx_decl = @@ -363,10 +367,12 @@ nullptr, lldb::eAccessPublic, "EmptyDerived2", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived2); - CXXBaseSpecifier *non_empty_vbase_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, true, false); - result = m_ast->SetBaseClassesForClassType(empty_derived2.GetOpaqueQualType(), - &non_empty_vbase_spec, 1); + std::unique_ptr non_empty_vbase_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, true, false); + bases.push_back(std::move(non_empty_vbase_spec)); + result = m_ast->TransferBaseClasses(empty_derived2.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived2); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_vbase_cxx_decl = @@ -377,9 +383,6 @@ empty_derived_non_empty_vbase_cxx_decl, false)); EXPECT_TRUE( ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl)); - - delete non_empty_base_spec; - delete non_empty_vbase_spec; } TEST_F(TestClangASTContext, TemplateArguments) { Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp === --- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -59,12 +59,12 @@ CVType udt_cvt = m_symbol_file.m_index->tpi().getType(ti); lldb::opaque_compiler_type_t base_qt = base_ct.GetOpaqueQualType(); - clang::CXXBaseSpecifier *base_spec = + std::unique_ptr base_spec = m_symbol_file.GetASTContext().CreateBaseClassSpecifier( base_qt, TranslateMemberAccess(access), false, udt_cvt.kind() == LF_CLASS); - - m_bases.push_back(base_spec); + lldbassert(base_spec); + m_bases.push_back(std::move(base_spec)); return base_qt; } @@ -172,9 +172,8 @@ void UdtRecordCompleter::complete() { ClangASTContext &clang = m_symbol_file.GetASTContext(); - clang.SetBaseClassesForClassType(m_derived_ct.GetOpaqueQualType(), - m_bases.data(), m_bases.size()); - ClangASTContext::DeleteBaseClassSpecifiers(m_bases.data(), m_bases.size()); + clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), +std::move(m_bases)); clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQual
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB345313: Don't type-erase the SymbolContextItem enumeration. (authored by zturner, committed by ). Herald added a subscriber: abidh. Changed prior to commit: https://reviews.llvm.org/D53597?vs=170737&id=171187#toc Repository: rLLDB LLDB https://reviews.llvm.org/D53597 Files: include/lldb/Core/Address.h include/lldb/Core/Module.h include/lldb/Core/ModuleList.h include/lldb/Symbol/CompileUnit.h include/lldb/Symbol/SymbolFile.h include/lldb/Symbol/SymbolVendor.h include/lldb/Target/StackFrame.h include/lldb/lldb-enumerations.h source/API/SBAddress.cpp source/API/SBFrame.cpp source/API/SBModule.cpp source/API/SBTarget.cpp source/Commands/CommandObjectSource.cpp source/Core/Address.cpp source/Core/Disassembler.cpp source/Core/Module.cpp source/Core/ModuleList.cpp source/Core/SourceManager.cpp source/Plugins/Architecture/Mips/ArchitectureMips.cpp source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp source/Plugins/Process/Utility/RegisterContextLLDB.cpp source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.h source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h source/Symbol/CompileUnit.cpp source/Symbol/SymbolFile.cpp source/Symbol/SymbolVendor.cpp source/Target/StackFrame.cpp Index: source/API/SBModule.cpp === --- source/API/SBModule.cpp +++ source/API/SBModule.cpp @@ -217,9 +217,9 @@ uint32_t resolve_scope) { SBSymbolContext sb_sc; ModuleSP module_sp(GetSP()); + SymbolContextItem scope = static_cast(resolve_scope); if (module_sp && addr.IsValid()) -module_sp->ResolveSymbolContextForAddress(addr.ref(), resolve_scope, - *sb_sc); +module_sp->ResolveSymbolContextForAddress(addr.ref(), scope, *sb_sc); return sb_sc; } Index: source/API/SBTarget.cpp === --- source/API/SBTarget.cpp +++ source/API/SBTarget.cpp @@ -660,11 +660,12 @@ SBTarget::ResolveSymbolContextForAddress(const SBAddress &addr, uint32_t resolve_scope) { SBSymbolContext sc; + SymbolContextItem scope = static_cast(resolve_scope); if (addr.IsValid()) { TargetSP target_sp(GetSP()); if (target_sp) - target_sp->GetImages().ResolveSymbolContextForAddress( - addr.ref(), resolve_scope, sc.ref()); + target_sp->GetImages().ResolveSymbolContextForAddress(addr.ref(), scope, +sc.ref()); } return sc; } Index: source/API/SBAddress.cpp === --- source/API/SBAddress.cpp +++ source/API/SBAddress.cpp @@ -198,8 +198,9 @@ SBSymbolContext SBAddress::GetSymbolContext(uint32_t resolve_scope) { SBSymbolContext sb_sc; + SymbolContextItem scope = static_cast(resolve_scope); if (m_opaque_ap->IsValid()) -m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), resolve_scope); +m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), scope); return sb_sc; } Index: source/API/SBFrame.cpp === --- source/API/SBFrame.cpp +++ source/API/SBFrame.cpp @@ -112,16 +112,16 @@ SBSymbolContext sb_sym_ctx; std::unique_lock lock; ExecutionContext exe_ctx(m_opaque_sp.get(), lock); - + SymbolContextItem scope = static_cast(resolve_scope); StackFrame *frame = nullptr; Target *target = exe_ctx.GetTargetPtr(); Process *process = exe_ctx.GetProcessPtr(); if (target && process) { Process::StopLocker stop_locker; if (stop_locker.TryLock(&process->GetRunLock())) { frame = exe_ctx.GetFramePtr(); if (frame) { -sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(resolve_scope)); +sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(scope)); } else { if (log) log->Printf("SBFrame::GetVariables () => error: could not " Index: source/Core/SourceManager.cpp === --- source/Core/SourceManager.cpp +++ source/Core/SourceManager
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL345313: Don't type-erase the SymbolContextItem enumeration. (authored by zturner, committed by ). Herald added subscribers: llvm-commits, jrtc27. Changed prior to commit: https://reviews.llvm.org/D53597?vs=170737&id=171186#toc Repository: rL LLVM https://reviews.llvm.org/D53597 Files: lldb/trunk/include/lldb/Core/Address.h lldb/trunk/include/lldb/Core/Module.h lldb/trunk/include/lldb/Core/ModuleList.h lldb/trunk/include/lldb/Symbol/CompileUnit.h lldb/trunk/include/lldb/Symbol/SymbolFile.h lldb/trunk/include/lldb/Symbol/SymbolVendor.h lldb/trunk/include/lldb/Target/StackFrame.h lldb/trunk/include/lldb/lldb-enumerations.h lldb/trunk/source/API/SBAddress.cpp lldb/trunk/source/API/SBFrame.cpp lldb/trunk/source/API/SBModule.cpp lldb/trunk/source/API/SBTarget.cpp lldb/trunk/source/Commands/CommandObjectSource.cpp lldb/trunk/source/Core/Address.cpp lldb/trunk/source/Core/Disassembler.cpp lldb/trunk/source/Core/Module.cpp lldb/trunk/source/Core/ModuleList.cpp lldb/trunk/source/Core/SourceManager.cpp lldb/trunk/source/Plugins/Architecture/Mips/ArchitectureMips.cpp lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/trunk/source/Symbol/CompileUnit.cpp lldb/trunk/source/Symbol/SymbolFile.cpp lldb/trunk/source/Symbol/SymbolVendor.cpp lldb/trunk/source/Target/StackFrame.cpp Index: lldb/trunk/source/Symbol/CompileUnit.cpp === --- lldb/trunk/source/Symbol/CompileUnit.cpp +++ lldb/trunk/source/Symbol/CompileUnit.cpp @@ -278,7 +278,8 @@ uint32_t CompileUnit::ResolveSymbolContext(const FileSpec &file_spec, uint32_t line, bool check_inlines, - bool exact, uint32_t resolve_scope, + bool exact, + SymbolContextItem resolve_scope, SymbolContextList &sc_list) { // First find all of the file indexes that match our "file_spec". If // "file_spec" has an empty directory, then only compare the basenames when Index: lldb/trunk/source/Symbol/SymbolVendor.cpp === --- lldb/trunk/source/Symbol/SymbolVendor.cpp +++ lldb/trunk/source/Symbol/SymbolVendor.cpp @@ -235,7 +235,7 @@ } uint32_t SymbolVendor::ResolveSymbolContext(const Address &so_addr, -uint32_t resolve_scope, +SymbolContextItem resolve_scope, SymbolContext &sc) { ModuleSP module_sp(GetModule()); if (module_sp) { @@ -248,7 +248,7 @@ uint32_t SymbolVendor::ResolveSymbolContext(const FileSpec &file_spec, uint32_t line, bool check_inlines, -uint32_t resolve_scope, +SymbolContextItem resolve_scope, SymbolContextList &sc_list) { ModuleSP module_sp(GetModule()); if (module_sp) { Index: lldb/trunk/source/Symbol/SymbolFile.cpp === --- lldb/trunk/source/Symbol/SymbolFile.cpp +++ lldb/trunk/source/Symbol/SymbolFile.cpp @@ -97,7 +97,7 @@ uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec, uint32_t line, bool check_inlines, - uint32_t resolve_scope, + lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list) { return 0; } Index: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.
[Lldb-commits] [PATCH] D53616: Don't type-erase the FunctionNameType or TypeClass enums
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL345314: Don't type-erase the FunctionNameType or TypeClass enums. (authored by zturner, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53616?vs=170762&id=171188#toc Repository: rL LLVM https://reviews.llvm.org/D53616 Files: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h lldb/trunk/include/lldb/Core/Module.h lldb/trunk/include/lldb/Core/ModuleList.h lldb/trunk/include/lldb/Symbol/SymbolFile.h lldb/trunk/include/lldb/Symbol/SymbolVendor.h lldb/trunk/include/lldb/Target/Target.h lldb/trunk/include/lldb/lldb-enumerations.h lldb/trunk/source/API/SBCompileUnit.cpp lldb/trunk/source/API/SBModule.cpp lldb/trunk/source/API/SBTarget.cpp lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp lldb/trunk/source/Core/Module.cpp lldb/trunk/source/Core/ModuleList.cpp lldb/trunk/source/Expression/IRExecutionUnit.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/trunk/source/Symbol/SymbolFile.cpp lldb/trunk/source/Symbol/SymbolVendor.cpp lldb/trunk/source/Target/Target.cpp Index: lldb/trunk/source/Expression/IRExecutionUnit.cpp === --- lldb/trunk/source/Expression/IRExecutionUnit.cpp +++ lldb/trunk/source/Expression/IRExecutionUnit.cpp @@ -709,9 +709,10 @@ struct IRExecutionUnit::SearchSpec { ConstString name; - uint32_t mask; + lldb::FunctionNameType mask; - SearchSpec(ConstString n, uint32_t m = lldb::eFunctionNameTypeFull) + SearchSpec(ConstString n, + lldb::FunctionNameType m = lldb::eFunctionNameTypeFull) : name(n), mask(m) {} }; Index: lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp === --- lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp +++ lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp @@ -30,7 +30,7 @@ using namespace lldb_private; BreakpointResolverName::BreakpointResolverName( -Breakpoint *bkpt, const char *name_cstr, uint32_t name_type_mask, +Breakpoint *bkpt, const char *name_cstr, FunctionNameType name_type_mask, LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset, bool skip_prologue) : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset), @@ -51,7 +51,7 @@ BreakpointResolverName::BreakpointResolverName( Breakpoint *bkpt, const char *names[], size_t num_names, -uint32_t name_type_mask, LanguageType language, lldb::addr_t offset, +FunctionNameType name_type_mask, LanguageType language, lldb::addr_t offset, bool skip_prologue) : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset), m_match_type(Breakpoint::Exact), m_language(language), @@ -61,9 +61,12 @@ } } -BreakpointResolverName::BreakpointResolverName( -Breakpoint *bkpt, std::vector names, uint32_t name_type_mask, -LanguageType language, lldb::addr_t offset, bool skip_prologue) +BreakpointResolverName::BreakpointResolverName(Breakpoint *bkpt, + std::vector names, + FunctionNameType name_type_mask, + LanguageType language, + lldb::addr_t offset, + bool skip_prologue) : BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset), m_match_type(Breakpoint::Exact), m_language(language), m_skip_prologue(skip_prologue) { @@ -161,23 +164,23 @@ return nullptr; } std::vector names; -std::vector name_masks; +std::vector name_masks; for (size_t i = 0; i < num_elem; i++) { - uint32_t name_mask; llvm::StringRef name; success = names_array->GetItemAtIndexAsString(i, name); if (!success) { error.SetErrorString("BRN::CFSD: name entry is not a string."); return nullptr; } - success = names_mask_array->GetItemAtIndexAsInteger(i, name_mask); + std::underlying_type::type fnt
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner created this revision. zturner added reviewers: lemo, aleksandr.urakov, stella.stamenova. LLDB has the ability to display global variables, even without a running process, via the `target variable` command. This is because global variables are linker initialized, so their values are embedded directly into the executables. This gives us great power for testing native PDB functionality in a cross-platform manner, because we don't actually need a running process. We can just create a target using an EXE file, and display global variables. And global variables can have arbitrarily complex types, so in theory we can fully exercise the type system, record layout, and data formatters for native PDB files and PE/COFF executables on any host platform, as long as our type does not require a dynamic initializer. This patch adds basic support for finding variables by name, and adds an exhaustive test for fundamental data types and pointers / references to fundamental data types. Subsequent patches will extend this to typedefs, classes, pointers to functions, and other cases. https://reviews.llvm.org/D53731 Files: lldb/lit/SymbolFile/NativePDB/Inputs/globals-fundamental.lldbinit lldb/lit/SymbolFile/NativePDB/globals-fundamental.cpp lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -103,6 +103,11 @@ size_t ParseFunctionBlocks(const SymbolContext &sc) override; + uint32_t FindGlobalVariables(const ConstString &name, + const CompilerDeclContext *parent_decl_ctx, + uint32_t max_matches, + VariableList &variables) override; + size_t ParseTypes(const SymbolContext &sc) override; size_t ParseVariablesForContext(const SymbolContext &sc) override { return 0; @@ -175,11 +180,13 @@ lldb::CompUnitSP GetOrCreateCompileUnit(const CompilandIndexItem &cci); lldb::TypeSP GetOrCreateType(PdbSymUid type_uid); lldb::TypeSP GetOrCreateType(llvm::codeview::TypeIndex ti); + lldb::VariableSP GetOrCreateGlobalVariable(PdbSymUid var_uid); lldb::FunctionSP CreateFunction(PdbSymUid func_uid, const SymbolContext &sc); lldb::CompUnitSP CreateCompileUnit(const CompilandIndexItem &cci); lldb::TypeSP CreateType(PdbSymUid type_uid); lldb::TypeSP CreateAndCacheType(PdbSymUid type_uid); + lldb::VariableSP CreateGlobalVariable(PdbSymUid var_uid); llvm::BumpPtrAllocator m_allocator; @@ -193,6 +200,7 @@ llvm::DenseMap m_uid_to_decl; + llvm::DenseMap m_global_vars; llvm::DenseMap m_functions; llvm::DenseMap m_compilands; llvm::DenseMap m_types; Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -16,14 +16,17 @@ #include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/StreamBuffer.h" #include "lldb/Symbol/ClangASTContext.h" #include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/ClangExternalASTSourceCommon.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/LineTable.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Symbol/SymbolVendor.h" +#include "lldb/Symbol/Variable.h" +#include "lldb/Symbol/VariableList.h" #include "llvm/DebugInfo/CodeView/CVRecord.h" #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h" @@ -886,6 +889,128 @@ return CreateAndCacheType(type_uid); } +static DWARFExpression MakeGlobalLocationExpression(uint16_t section, +uint32_t offset, +ModuleSP module) { + if (!module) +return DWARFExpression(nullptr); + + const ArchSpec &architecture = module->GetArchitecture(); + ByteOrder byte_order = architecture.GetByteOrder(); + uint32_t address_size = architecture.GetAddressByteSize(); + uint32_t byte_size = architecture.GetDataByteSize(); + if (byte_order == eByteOrderInvalid || address_size == 0) +return DWARFExpression(nullptr); + + RegisterKind register_kind = eRegisterKindDWARF; + StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order); + stream.PutHex8(DW_OP_addr); + + SectionList *section_list = module->GetSectionList(); + if (!section_list) +return DWARFExpression(nullptr); + + uint32_t section_idx = section - 1; + if (section_idx >= section_list->GetSize()) +return DWA
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1276253, @jingham wrote: > So the current approach relies on the ability to sniff the name to determine > the context in which the user intends to find it. It does (and always did) > use the presence of an initial "::" to tell whether a symbol is exact. > That's obviously also inappropriate for a generic Type method. But OTOH, > there needs to be a funnel point where you take in a string you know nothing > about (from the user either in type lookup or in an expression) and find it > as best you can. I don't think we want to force command line users to say > "type lookup --exact " so we've got to figure it out internally. > > Internally, it might be helpful to do some initial analysis of the input type > string, and then dispatch it to an exact or inexact search. But given I > don't think you can get away without a FindTypes that takes a string that you > don't know whether it is exact or not, I don't feel strongly about that. > > We should abstract "is exact" and ask the various type systems to handle that > request, and we also need to abstract "remove type class" and again ask the > various type systems to handle that. That seems to me the main ugliness that > this patch reveals. Just to clarify, is the consensus then that I can submit this as is? Or that it needs some modification? Greg's suggestion of making a separate method called `FindExactType` could work, but it doesn't seem that much different than passing a boolean call `exact_match`, which is what I've done here. On the extreme end, we could just make `Module::FindTypes` do absolutely nothing except call the SymbolFile plugin.I don't feel too strongly either way. The current patch seems NFC for DWARF and strictly better for PDB, but I'm willing to make changes if anyone feels like there's an architecturally more sound approach. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. which generic SymbolFile test do you mean? The lit ones are the only ones that are set up to run in this particular manner (run lines, check lines, etc), and currently we don't have a way to run different / multiple command line invocations. I came up with this test in order to test the new changes I introduced in `SymbolFileNativePdb.cpp` that are also in this patch, so I definitely want to make sure all of that code still gets exercised with whatever test strategy I end up using. I guess the way I see it, there's two interesting things this test could exercise. 1) the debug info, and 2) the formatters. If we want to test the formatters, we could probably brainstorm a way to do it generically with 1 test (or set of tests) that omits the SymbolFile from the picture entirely and is generic enough that it should be applicable to any platform/compiler/host. If we want to test the SymbolFile plugin though, then it may need to be specific to the debug info type. I think what you're getting at though is that we probably need some notion of "debug info variants" for these lit tests like we have in the dotest suite. I actually had an idea for something that might work like that a while back, which I described here: https://reviews.llvm.org/D52618#1252906 But the idea would basically be that instead of hardcoding a command line like I've done with `clang-cl /Z7 etc etc` we could write something more generic that would evaluate to different (or perhaps even multiple) things, so we could run it different ways. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. In https://reviews.llvm.org/D53731#1276633, @jingham wrote: > Well, what's really going on is that I'm not familiar enough with lit to know > that it doesn't have the ability to run different commands to produce the > input file... But as you guessed, my point is that you have written a bunch > of tests that would be valuable to test against any symfile format, so it is > a shame to only run them against one format. OTOH, if that's not possible > right now, I'm content to wait for some enhancements to lit that make it > possible. I can maybe hardcode multiple runlines, one for each possible target triple. But what's really going on is that I'm not familiar enough with other platforms to know how to generate their binaries :) But I'll actually give it a try. Just to explain what's going on here, I've got these two lines: // RUN: clang-cl /Z7 /GS- /GR- /c /Fo%t.obj -- %s // RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj Those two run the compiler and linker and write the output to a temporary file, but don't actually check anything yet. The next two lines (which are actually one command broken with a continuation character) // RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \ // RUN: %p/Inputs/globals-fundamental.lldbinit | FileCheck %s runs lldb and pipes the result to FileCheck, using this file as the check file. So what I could *try* doing is having multiple compiler / linker invocations. One that produces a Windows executable / PDB file. One that produces a Linux executable / DWARF. One that produces some kind of OSX binary (I think lld doesn't work with MachO yet, so I'm not sure about this one), writing the linker outputs to different files each time. Then we could maybe run lldb 3 times, once against each input, but using the same check file each time. I'll give it a try and see what happens. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added reviewers: vsk, labath. zturner added a comment. In https://reviews.llvm.org/D53731#1276660, @jingham wrote: > Could you also use Vedant's new FileCheck dotest test class? That should > allow you to write the tests exactly as you are, but use the dotest mechanism > to build and run the example. Adding Vedant here. My understanding is that the work he did there is like the inverse of what I'm doing. It allows you to use FileCheck from inside of dotest tests, but I was trying to bypass dotest here. I don't necessarily think "dotest for all things" should be an explicit goal (i actually think long term we should move away from it, but that's for another day). Aside from that though, I don't think this particular test needs any of the functionality that dotest offers. It offers building in multiple configurations, but we can get that from this test by just specifying mulitple run lines (I'm testing this out locally and it seems like I can get it to work). Eventually, using some kind of configuration / builder type system like I brainstormed in the review thread I linked previously, I think we could have all the advantages of dotest's builder even with this style of test. FWIW, I was also under the impression that Vedant's solution with FileCheck in dotest was only intended as sort of an immediate solution to a problem, but not necessary the long term desired end-state. (I could be wrong about this though). https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. In https://reviews.llvm.org/D53731#1276743, @vsk wrote: > In https://reviews.llvm.org/D53731#1276732, @zturner wrote: > > > In https://reviews.llvm.org/D53731#1276660, @jingham wrote: > > > > > Could you also use Vedant's new FileCheck dotest test class? That should > > > allow you to write the tests exactly as you are, but use the dotest > > > mechanism to build and run the example. > > > > > > Adding Vedant here. My understanding is that the work he did there is like > > the inverse of what I'm doing. It allows you to use FileCheck from inside > > of dotest tests, but I was trying to bypass dotest here. I don't > > necessarily think "dotest for all things" should be an explicit goal (i > > actually think long term we should move away from it, but that's for > > another day). Aside from that though, I don't think this particular test > > needs any of the functionality that dotest offers. It offers building in > > multiple configurations, but we can get that from this test by just > > specifying mulitple run lines (I'm testing this out locally and it seems > > like I can get it to work). Eventually, using some kind of configuration / > > builder type system like I brainstormed in the review thread I linked > > previously, I think we could have all the advantages of dotest's builder > > even with this style of test. > > > > FWIW, I was also under the impression that Vedant's solution with FileCheck > > in dotest was only intended as sort of an immediate solution to a problem, > > but not necessary the long term desired end-state. (I could be wrong about > > this though). > > > The tests as-written seem to exercise the new functionality. I think it'd be > fine to use the FileCheck-in-{dotest,lldbinline} support if you need to set a > breakpoint, run a command, and then validate its output (though it looks like > you don't) Yea, once I need to actually run a process I expect I'll need to start using it. For the time being, I'm trying see how much ground I can cover without a process. The reasoning is two-fold. First, it means the tests can run anywhere. If there's no process, my test coverage isn't limited to a specific host OS. Second, from a layering perspective, if I know that there's a ton of test coverage for "everything that happens before a process is spawned", then once you get to the point that you are spawning process, it limits the scope of where you have to search when something does go wrong. So that's my thinking. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. In https://reviews.llvm.org/D53731#1276818, @jingham wrote: > dotest tests don't require a process. Presumably dotest knows how to build > windows targeted PDB debug flavor files (to go along with dwarf/dsym/etc.). > So it would be straightforward to make a test that had your input sources, > built and made a target out of it and then poked at static variables and > their types. That would straightaway run with all the different symbol file > formats we support. > > That was why using Vedant's FileCheck thing made sense to me. You would use > that to specify the test cases, since you like that way of writing tests and > anyway already have them written out in that form, but use the dotest > machinery to build it for whatever symfile format and target architecture/s > the person who was running the test dialed up. But if you are interesting in > also getting this to work with the straight FileCheck style test, your time > is your own... > > BTW, I would use dotest tests specifically for the kind of thing you are > doing here because then you can test against the SBType and SBTypeMembers > from the debug info you've ingested, which would give you bit and byte > offsets and sizes for free. But if your differing tastes end up getting you > to add that info to "type lookup" - which we really should do - then I guess > we win either way... Yea, so one of the commands I'm used to on the Windows command line debugger WinDbg is the `dt` (dump type) command. It's basically the equivalent of `type lookup` - more powerful in some ways, less in others . The output format looks like this: 0:000> dt _EXCEPTION_RECORD ntdll!_EXCEPTION_RECORD +0x000 ExceptionCode: Int4B +0x004 ExceptionFlags : Uint4B +0x008 ExceptionRecord : Ptr64 _EXCEPTION_RECORD +0x010 ExceptionAddress : Ptr64 Void +0x018 NumberParameters : Uint4B +0x020 ExceptionInformation : [15] Uint8B So you can see the field offsets and such. (Less powerful because that's about it, no methods, for example). But you can also pass it an address, which it then formats the block of memory as that type. e.g. 0:000> dt ntdll!_TEB32 007ed000 +0x000 NtTib: _NT_TIB32 +0x01c EnvironmentPointer : 0 +0x020 ClientId : _CLIENT_ID32 +0x028 ActiveRpcHandle : 0 +0x02c ThreadLocalStoragePointer : 0 +0x030 ProcessEnvironmentBlock : 0x7ed000 +0x034 LastErrorValue : 0 +0x038 CountOfOwnedCriticalSections : 0 +0x03c CsrClientThread : 0 +0x040 Win32ThreadInfo : 0x4d18 +0x044 User32Reserved : [26] 0 +0x0ac UserReserved : [5] 0 So up on my list of things to implement is these two features. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h:46 + // due to the debug magic at the beginning of the stream. + uint64_t global : 1; // True if this is from the globals stream. + uint64_t modi : 16; // For non-global, this is the 0-based index of module. lemo wrote: > 30 + 1 != 32 - what's going on? There's supposed to be a `uint64_t unused : 1;` here. Thanks for noticing. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:913 + + uint32_t section_idx = section - 1; + if (section_idx >= section_list->GetSize()) lemo wrote: > comment explaining the - 1 ? I'm going to be totally honest here. I don't understand this code at all. I copied it from the `SymbolFilePDB` implementation. It seems to work :-/ Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:936 + lldb::ValueType scope = eValueTypeInvalid; + TypeIndex ti; + llvm::StringRef name; lemo wrote: > pls explicitly initialize all the locals `TypeIndex` has a constructor, but I should definitely initialize the `lldb::addr_t` Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1406 + TypeSP type_sp = CreateAndCacheType(uid); + return &*type_sp; } lemo wrote: > type_sp->get() seems cleaner / more readable You know, I kept trying to write that, and I was like "what silly person on the C++ standards committee forgot to put a `get()` method on `std::shared_ptr<>`. Did they really forget this or am I just going crazy?" Turns out it's a bug in MSVC intellisense where it doesn't show it for some reason. So I took that to mean it didn't exist. Shame on me. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:913 + + uint32_t section_idx = section - 1; + if (section_idx >= section_list->GetSize()) zturner wrote: > lemo wrote: > > comment explaining the - 1 ? > I'm going to be totally honest here. I don't understand this code at all. I > copied it from the `SymbolFilePDB` implementation. It seems to work :-/ Ok, I thought about this some, it's actually pretty simple. I'll add a comment. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
This revision was automatically updated to reflect the committed changes. Closed by commit rL345373: [NativePDB] Add the ability to dump dump global variables. (authored by zturner, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53731?vs=171189&id=171268#toc Repository: rL LLVM https://reviews.llvm.org/D53731 Files: lldb/trunk/lit/SymbolFile/NativePDB/Inputs/globals-fundamental.lldbinit lldb/trunk/lit/SymbolFile/NativePDB/globals-fundamental.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -16,14 +16,17 @@ #include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/StreamBuffer.h" #include "lldb/Symbol/ClangASTContext.h" #include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/ClangExternalASTSourceCommon.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/LineTable.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Symbol/SymbolVendor.h" +#include "lldb/Symbol/Variable.h" +#include "lldb/Symbol/VariableList.h" #include "llvm/DebugInfo/CodeView/CVRecord.h" #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h" @@ -887,6 +890,131 @@ return CreateAndCacheType(type_uid); } +static DWARFExpression MakeGlobalLocationExpression(uint16_t section, +uint32_t offset, +ModuleSP module) { + assert(section > 0); + assert(module); + + const ArchSpec &architecture = module->GetArchitecture(); + ByteOrder byte_order = architecture.GetByteOrder(); + uint32_t address_size = architecture.GetAddressByteSize(); + uint32_t byte_size = architecture.GetDataByteSize(); + assert(byte_order != eByteOrderInvalid && address_size != 0); + + RegisterKind register_kind = eRegisterKindDWARF; + StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order); + stream.PutHex8(DW_OP_addr); + + SectionList *section_list = module->GetSectionList(); + assert(section_list); + + // Section indices in PDB are 1-based, but in DWARF they are 0-based, so we + // need to subtract 1. + uint32_t section_idx = section - 1; + if (section_idx >= section_list->GetSize()) +return DWARFExpression(nullptr); + + auto section_ptr = section_list->GetSectionAtIndex(section_idx); + if (!section_ptr) +return DWARFExpression(nullptr); + + stream.PutMaxHex64(section_ptr->GetFileAddress() + offset, address_size, + byte_order); + DataBufferSP buffer = + std::make_shared(stream.GetData(), stream.GetSize()); + DataExtractor extractor(buffer, byte_order, address_size, byte_size); + DWARFExpression result(module, extractor, nullptr, 0, buffer->GetByteSize()); + result.SetRegisterKind(register_kind); + return result; +} + +VariableSP SymbolFileNativePDB::CreateGlobalVariable(PdbSymUid var_uid) { + const PdbCuSymId &cu_sym = var_uid.asCuSym(); + lldbassert(cu_sym.global); + CVSymbol sym = m_index->symrecords().readRecord(cu_sym.offset); + lldb::ValueType scope = eValueTypeInvalid; + TypeIndex ti; + llvm::StringRef name; + lldb::addr_t addr = 0; + uint16_t section = 0; + uint32_t offset = 0; + bool is_external = false; + switch (sym.kind()) { + case S_GDATA32: +is_external = true; +LLVM_FALLTHROUGH; + case S_LDATA32: { +DataSym ds(sym.kind()); +llvm::cantFail(SymbolDeserializer::deserializeAs(sym, ds)); +ti = ds.Type; +scope = (sym.kind() == S_GDATA32) ? eValueTypeVariableGlobal + : eValueTypeVariableStatic; +name = ds.Name; +section = ds.Segment; +offset = ds.DataOffset; +addr = m_index->MakeVirtualAddress(ds.Segment, ds.DataOffset); +break; + } + case S_GTHREAD32: +is_external = true; +LLVM_FALLTHROUGH; + case S_LTHREAD32: { +ThreadLocalDataSym tlds(sym.kind()); +llvm::cantFail( +SymbolDeserializer::deserializeAs(sym, tlds)); +ti = tlds.Type; +name = tlds.Name; +section = tlds.Segment; +offset = tlds.DataOffset; +addr = m_index->MakeVirtualAddress(tlds.Segment, tlds.DataOffset); +scope = eValueTypeVariableThreadLocal; +break; + } + default: +llvm_unreachable("unreachable!"); + } + + CompUnitSP comp_unit; + llvm::Optional modi = m_index->GetModuleIndexForVa(addr); + if (modi) { +PdbSymUid cuid = PdbSymUid::makeCompilandId(*modi); +CompilandIndexItem &cci = m_index->compilands().GetOrCreateCompiland(cuid); +comp_unit = Ge
[Lldb-commits] [PATCH] D53749: [PDB] Fix `SymbolFilePDBTests` after r345313
zturner added a subscriber: aleksandr.urakov. zturner added a comment. Ahh, I meant to remind you, because I noticed this when I was looking through the SymbolFilePDB code recently. I think the LLVM guideline is not to use so much auto. The generally accepted rule is that we can use for the result of make_shared, make_unique, and result of iterator types, but otherwise we prefer to explicitly spell the types out. You don't have to go and submit another patch on top of this one to fix it again, it's just something to keep in mind. That said, thanks for fixing this for me. Sorry for the break. Repository: rLLDB LLDB https://reviews.llvm.org/D53749 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53749: [PDB] Fix `SymbolFilePDBTests` after r345313
zturner added a comment. For trivial changes it's ok to submit without review. This is true for cleanup and trivial refactor, but especially for build break like this one., and even more so if you consider yourself code owner in the corresponding code area. Repository: rLLDB LLDB https://reviews.llvm.org/D53749 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53753: [Windows] Define generic arguments registers for Windows x64
zturner added a subscriber: aleksandr.urakov. zturner added a comment. Lgtm Repository: rLLDB LLDB https://reviews.llvm.org/D53753 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a subscriber: clayborg. zturner added a comment. Ok, that reasoning makes sense, I’ll see what i can do https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec
zturner accepted this revision. zturner added a comment. I always wondered if we actually even need methods like this in `FileSystem` given that they already exist in `llvm::sys::fs`. Is it possible to just call the llvm methods directly, or is it still helpful to call the ones in `FileSystem` so we can more transparently interoperate with the VFS layer somehow? Repository: rLLDB LLDB https://reviews.llvm.org/D53788 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. Is this still blocked on anything? https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52618: [Windows] A basic implementation of memory allocations in a debuggee process
zturner added a comment. I think we should try as hard as possible to have just one way of writing these tests. So if we know there are going to be two different use cases, one where we have mutually exclusive variants (e.g. run a process on OSX, Linux, Windows), and platform agnostic variants (compile for multiple platform / debug info variants, run tests in every possible configuration), I think we should try to unify all of these under a single syntax. One possible syntax that works today is to duplicate all the compile and test lines but delegate to an external check file. ; file 1 ; REQUIRES: windows ; RUN: clang-cl ... ; RUN: lld-link ... ; RUN: lldb-test | FileCheck --check-file=%p/Inputs/some-test.check ; file 2 ; UNSUPPORTED: windows ; RUN: clang++ ... ; RUN: lldb-test | FileCheck --check-file=%p/Inputs/some-test.check This unblocks the patch, doesn't require any fancy logic that would be unfamiliar with people coming to LLDB, and is pretty simple. I think we can do better than the above by removing the need to have multiple files, but the substitutions only get us halfway there because they don't handle the case where we actually want to repeat the same test multiple times with different command lines. So since we're going to have to fall back to the above approach for that, let's just do it everywhere until we can come up with a solution that unifies everything under one syntax. FWIW, I've been brainstorming some modifications to the core lit infrastructure that would support this kind of thing. Here's some strawman syntax: VARIANTS: v1, v2 REQUIRES-v1: windows UNSUPPORTED-v2: windows RUN-v1: %cxx %p/Inputs/call-function.cpp -g -o %t RUN-v2: clang-cl /Zi %p/Inputs/call-function.cpp -o %t RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-basic RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1 RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-overlap1 RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-mix-malloc-free RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-mix-malloc-free Now in this case the two variants are mutually exclusive, but they need not be. So, for example, in the case of my target variables test for builtin types, which I wrote as this: // Test that we can display tag types. // RUN: clang-cl /Z7 /GS- /GR- /c -Xclang -fkeep-static-consts /Fo%t.obj -- %s // RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj // RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \ // RUN: %p/Inputs/globals-fundamental.lldbinit | FileCheck %s // bunch of source code We could re-write it as: // VARIANTS: v1, v2 // RUN-v1: clang-cl /Z7 /GS- /GR- /c -Xclang -fkeep-static-consts /Fo%t.obj -- %s // RUN-v1: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj // RUN-v1: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \ // RUN-v1: %p/Inputs/globals-fundamental.lldbinit | FileCheck %s // RUN-v2: clang++ -g -m64 -Xclang -fkeep-static-consts --target=x86_x64-linux-gnu -o %t -- %s // RUN-v2: lldb -f %t -s %p/Inputs/globals-fundamental.lldbinit | FileCheck %s // bunch of source code This will run variant 1, then run variant 2. I think this fits nicely with the lit "philosophy" and handles all the use cases that we need. But there's a lot of detail in implementing it correctly (and right now it's just in my head, I'm not actually working on it). So I think we should go with the simple thing for now of just having 2 files and one shared check file so that we can unblock the patch and make forward progress https://reviews.llvm.org/D52618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53822: [NativePDB] Add tests for dumping global variables of class type
zturner created this revision. zturner added reviewers: lemo, aleksandr.urakov. Previous patches added support for dumpign global variables of builtin types, so this patch does the same for class types. For the most part, everything just worked, there was only one minor bug fix needed, which was that when a variable is of a modified class type (const, volatile, etc), we can't resolve the forward decl in `CreateAndCacheType`, so when it comes time to call `CompleteType`, one of our asserts was firing because we expected a Class, Struct, Enum, or Union, but we were getting an `LF_MODIFIER`. The other issue was that one of my tests added an array member, and we hadn't yet handled them, so I just went ahead and handled them since it was easy. There's probably room for lots of interesting test cases here, but these are the ones I was able to come up with. https://reviews.llvm.org/D53822 Files: lldb/lit/SymbolFile/NativePDB/Inputs/globals-classes.lldbinit lldb/lit/SymbolFile/NativePDB/global-classes.cpp lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -170,6 +170,8 @@ const llvm::codeview::EnumRecord &er); lldb::TypeSP CreateTagType(PdbSymUid type_uid, const llvm::codeview::UnionRecord &ur); + lldb::TypeSP CreateArrayType(PdbSymUid type_uid, + const llvm::codeview::ArrayRecord &ar); lldb::TypeSP CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, clang::TagTypeKind ttk, Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -391,9 +391,7 @@ // If this is an LF_MODIFIER, look through it to get the kind that it // modifies. Note that it's not possible to have an LF_MODIFIER that // modifies another LF_MODIFIER, although this would handle that anyway. - ModifierRecord mr; - llvm::cantFail(TypeDeserializer::deserializeAs(cvt, mr)); - return GetPdbSymType(tpi, mr.ModifiedType); + return GetPdbSymType(tpi, LookThroughModifierRecord(cvt)); } static clang::TagTypeKind TranslateUdtKind(const TagRecord &cr) { @@ -774,6 +772,25 @@ lldb_private::Type::eResolveStateForward); } +TypeSP SymbolFileNativePDB::CreateArrayType(PdbSymUid type_uid, +const ArrayRecord &ar) { + TypeSP element_type = GetOrCreateType(ar.ElementType); + uint64_t element_count = ar.Size / element_type->GetByteSize(); + + CompilerType element_ct = element_type->GetFullCompilerType(); + + CompilerType array_ct = + m_clang->CreateArrayType(element_ct, element_count, false); + + Declaration decl; + TypeSP array_sp = std::make_shared( + type_uid.toOpaqueId(), m_clang->GetSymbolFile(), ConstString(), ar.Size, + nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, + array_ct, lldb_private::Type::eResolveStateFull); + array_sp->SetEncodingType(element_type.get()); + return array_sp; +} + TypeSP SymbolFileNativePDB::CreateType(PdbSymUid type_uid) { const PdbTypeSymId &tsid = type_uid.asTypeSym(); TypeIndex index(tsid.index); @@ -816,6 +833,12 @@ return CreateTagType(type_uid, ur); } + if (cvt.kind() == LF_ARRAY) { +ArrayRecord ar; +llvm::cantFail(TypeDeserializer::deserializeAs(cvt, ar)); +return CreateArrayType(type_uid, ar); + } + return nullptr; } @@ -1441,6 +1464,25 @@ auto types_iter = m_types.find(uid.toOpaqueId()); lldbassert(types_iter != m_types.end()); + if (cvt.kind() == LF_MODIFIER) { +TypeIndex unmodified_type = LookThroughModifierRecord(cvt); +cvt = m_index->tpi().getType(unmodified_type); +// LF_MODIFIERS usually point to forward decls, so this is the one case +// where we won't have been able to resolve a forward decl to a full decl +// earlier on. So we need to do that now. +if (IsForwardRefUdt(cvt)) { + llvm::Expected expected_full_ti = + m_index->tpi().findFullDeclForForwardRef(unmodified_type); + if (!expected_full_ti) { +llvm::consumeError(expected_full_ti.takeError()); +return false; + } + cvt = m_index->tpi().getType(*expected_full_ti); + lldbassert(!IsForwardRefUdt(cvt)); + unmodified_type = *expected_full_ti; +} +uid = PdbSymUid::makeTypeSymId(ui
[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec
zturner added a comment. In https://reviews.llvm.org/D53788#1279096, @shafik wrote: > Are we refactoring in the right place? Why not just refactor > `FileSpec::GetByteSize()` why is `FileSpec` the wrong place? There was another review thread where we discussed this just the other day. Basically, It took a herculean effort to get `FileSpec` into the Utility library, which helped greatly with layering and we don't want to undo this. In order to for `GetByteSize` to be able to use the VFS, it has to be able to use `FileSystem`, so either `FileSpec` has to move out of `Utility`, `FileSystem` has to move into `Utility`, or `GetByteSize` has to move out of `FileSpec`. `FileSystem` moving into `Utility` might actually be a worthwhile goal long term, but even putting that aside, I think `FileSpec` should just be a thing for manipulating paths. For example, it has the notion of manipulating paths which might not even make sense on the given host platform (e.g. it has a member variable which represents the path *syntax). You might be on a Windows host debugging a remove Linux target, for example, and your host needs to have some way to manipulate paths on the remote which use a non-host syntax. That's what `FileSpec` is supposed to be. It just deals with strings. Repository: rLLDB LLDB https://reviews.llvm.org/D53788 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. So I started looking into this, and things get tricky. If we're doing a lookup by fully qualified name, I would expect there to never be more than one match, but LLDB doesn't seem to hold this same assumption. For example, in `ItaniumABILanguageRuntime::GetTypeInfoFromVTableAddress` there is code that tries to get an exact match from a single module, and if it doesn't find one, then searches the entire module list for multiple matches on exact name, then specifically handles the case where more than one match was found. Most of this code is all just for the purposes of logging, but if we cut through all the logging and just get down to the code, it basically amounts to this: type_info.SetName(class_name); TypeList class_types; uint32_t num_matches = 0; // First look in the module that the vtable symbol came from and // look for a single exact match. llvm::DenseSet searched_symbol_files; if (sc.module_sp) { num_modules = sc.module_sp->FindTypes(sc, ConstString(lookup_name), exact_match, 1, searched_symbol_files, class_types); } // If we didn't find a symbol, then move on to the entire module // list in the target and get as many unique matches as possible if (num_matches == 0) { num_matches = target.GetImages().FindTypes( sc, ConstString(lookup_name), exact_match, UINT32_MAX, searched_symbol_files, class_types); } lldb::TypeSP type_sp; if (num_matches == 0) return TypeAndOrName(); if (num_matches == 1) { type_sp = class_types.GetTypeAtIndex(0); if (type_sp) { if (ClangASTContext::IsCXXClassType( type_sp->GetForwardCompilerType())) { type_info.SetTypeSP(type_sp); } } else if (num_matches > 1) { size_t i; for (i = 0; i < num_matches; i++) { type_sp = class_types.GetTypeAtIndex(i); if (type_sp) { if (ClangASTContext::IsCXXClassType( type_sp->GetForwardCompilerType())) { type_info.SetTypeSP(type_sp); } } } } if (type_info) SetDynamicTypeInfo(vtable_addr, type_info); return type_info; } Why would we ever get more than one type on a fully qualifed exact match name lookup? And even if we did, why would we care which one we chose? https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1279738, @jingham wrote: > Exact matches aren't a C++ specific thing. An exact match is useful for > mixed C/C++ since you might want to say Foo and not Bar::Foo. IIRC a couple > of the places where exact was dialed up explicitly in FindTypes were for C > types. But since C and ObjC allow multiple types with the same name, that's > one way you might see multiple matches for "exact match". Moreover C+ used > to be fuzzy about whether non-exported classes from different linkage uses > had to follow the ODR. I haven't followed whether this got nailed down but > it still seems Quixotic to expect you could enforce this, since you are > asking two totally unrelated library vendors to avoid each other's names for > private classes... So having several classes with the same name is still a > possibility IRL with C++. > > Why we care is that if somebody runs the command: > > (lldb) expression (::SomeClass *) 0x12345 > > We're going to try our best to make that work. If SomeClass is not visible > in the module/comp_unit/function in which we are currently stopped, then we > will go looking far and wide for SomeClass. If we find one result somewhere > out in the world, the expression will succeed, but if we find two results > which are different, then we want to give an error about ambiguous type. So when designing this new `FindTypesByFullyQualifiedName` API, is it safe to assume, for the purposes of the API signature, that at the //Module// level, it will either return 0 or 1 match (e.g. the return value shoudl be a `TypeSP`, but at the //ModuleList// level, it can return arbitrarily many (so the return value should be a `TypeList`? https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1279761, @jingham wrote: > That depends on the definition of "fully qualified name". If you can ensure > that it means "name of C++ class" - or other ODR enforcing type system, then > you could make that assumption. In C you are free to redefine types on a > per-function basis if you so desire; and sadly some interface generators > (including MIG the one for generating Mach message handlers) do just this...) > So you would need to see a way to restrict the inputs to this function. > That doesn't seem like it would be straightforward to me. After I've changed up the codebase there is only one usage of it that I'm not sure is used in an ODR-enforcing context, which is `ObjCLanguageRuntime::LookupInCompleteClassCache`. All other usages are C++ or Java specific. Anyway, I guess I'll do it that way for now and upload the patch. This already turned out to be more invasvive than I was hoping for, so if this doesn't work out or someone doesn't like the approach, maybe I can just go back to the original patch and submit that https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1279777, @jingham wrote: > Yes, that usage is exactly the sort of use I was talking about. When we get > an ObjC object and want to find its dynamic type, we read the type name by > following the object's ISA pointer to the Class object. Then we go to look > up the type from that name. We know we want an exact match to the name we > found, but there's nothing that says the same module can't have an C++ class > with the same name as an ObjC class. So that code needs to get all the types > found, and sort through them for the one that is an ObjC interface type, and > discard all the others. You will need to support that behavior somehow. So just to make sure I understand, The C++ and ObjC classes are both potentially in the same Module, both potentially with the same name, correct? https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53822: [NativePDB] Add tests for dumping global variables of class type
This revision was automatically updated to reflect the committed changes. Closed by commit rL345629: [NativePDB] Add support for dumping global variables of class type. (authored by zturner, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53822?vs=171537&id=171755#toc Repository: rL LLVM https://reviews.llvm.org/D53822 Files: lldb/trunk/lit/SymbolFile/NativePDB/Inputs/globals-classes.lldbinit lldb/trunk/lit/SymbolFile/NativePDB/global-classes.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.h lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.h === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.h +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.h @@ -59,6 +59,8 @@ lldb::AccessType TranslateMemberAccess(llvm::codeview::MemberAccess access); llvm::codeview::TypeIndex GetFieldListIndex(llvm::codeview::CVType cvt); +llvm::codeview::TypeIndex +LookThroughModifierRecord(llvm::codeview::CVType modifier); llvm::StringRef DropNameScope(llvm::StringRef name); Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -170,6 +170,8 @@ const llvm::codeview::EnumRecord &er); lldb::TypeSP CreateTagType(PdbSymUid type_uid, const llvm::codeview::UnionRecord &ur); + lldb::TypeSP CreateArrayType(PdbSymUid type_uid, + const llvm::codeview::ArrayRecord &ar); lldb::TypeSP CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, clang::TagTypeKind ttk, Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp @@ -343,6 +343,13 @@ } } +TypeIndex lldb_private::npdb::LookThroughModifierRecord(CVType modifier) { + lldbassert(modifier.kind() == LF_MODIFIER); + ModifierRecord mr; + llvm::cantFail(TypeDeserializer::deserializeAs(modifier, mr)); + return mr.ModifiedType; +} + llvm::StringRef lldb_private::npdb::DropNameScope(llvm::StringRef name) { // Not all PDB names can be parsed with CPlusPlusNameParser. // E.g. it fails on names containing `anonymous namespace'. Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -391,9 +391,7 @@ // If this is an LF_MODIFIER, look through it to get the kind that it // modifies. Note that it's not possible to have an LF_MODIFIER that // modifies another LF_MODIFIER, although this would handle that anyway. - ModifierRecord mr; - llvm::cantFail(TypeDeserializer::deserializeAs(cvt, mr)); - return GetPdbSymType(tpi, mr.ModifiedType); + return GetPdbSymType(tpi, LookThroughModifierRecord(cvt)); } static clang::TagTypeKind TranslateUdtKind(const TagRecord &cr) { @@ -775,6 +773,25 @@ lldb_private::Type::eResolveStateForward); } +TypeSP SymbolFileNativePDB::CreateArrayType(PdbSymUid type_uid, +const ArrayRecord &ar) { + TypeSP element_type = GetOrCreateType(ar.ElementType); + uint64_t element_count = ar.Size / element_type->GetByteSize(); + + CompilerType element_ct = element_type->GetFullCompilerType(); + + CompilerType array_ct = + m_clang->CreateArrayType(element_ct, element_count, false); + + Declaration decl; + TypeSP array_sp = std::make_shared( + type_uid.toOpaqueId(), m_clang->GetSymbolFile(), ConstString(), ar.Size, + nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, + array_ct, lldb_private::Type::eResolveStateFull); + array_sp->SetEncodingType(element_type.get()); + return array_sp; +} + TypeSP SymbolFileNativePDB::CreateType(PdbSymUid type_uid) { const PdbTypeSymId &tsid = type_uid.asTypeSym(); TypeIndex index(tsid.index); @@ -817,6 +834,12 @@ return CreateTagType(type_uid, ur); } + if (cvt.kind() == LF_ARRAY) { +ArrayRecord ar; +llvm::cantFail(TypeDeserializer::deserializeAs(cvt, ar)); +return CreateArrayType(type_uid, ar); + } + return nullptr; } @@ -1442,6 +1465,25 @@ auto types_iter = m_types.find(uid.toOpaqueId())
[Lldb-commits] [PATCH] D52461: [PDB] Introduce `MSVCUndecoratedNameParser`
zturner added a comment. In https://reviews.llvm.org/D52461#1280527, @aleksandr.urakov wrote: > Update the diff according to the discussion, making it possible to parse MSVC > demangled names by `CPlusPlusLanguage`. The old PDB plugin still uses > `MSVCUndecoratedNameParser` directly because: > > - we are sure that the name in PDB is an MSVC name; > - it has a more convenient interface, especially for restoring namespaces > from the parsed name. So I had an interesting solution to this while working on the native pdb plugin. it is impossible to use it with the old pdb plugin, but given that it works flawlessly for the native pdb plugin, depending on how urgent your need is, maybe you can just put off working on this until you're ready to move over to the native pdb plugin? Basically the idea is that the raw PDB contains mangled type names for every type. You can see this by dumping types using `llvm-pdbutil`, as follows (I just picked a random one from my build directory). D:\src\llvmbuild\ninja-x64>bin\llvm-pdbutil.exe dump -types bin\sancov.pdb | grep -A 2 LF_STRUCT | more 0x1001 | LF_STRUCTURE [size = 88] ``anonymous-namespace'::RawCoverage` unique name: `.?AURawCoverage@?A0xa74cdb40@@` vtable: , base list: , field list: -- 0x100A | LF_STRUCTURE [size = 212] `std::default_delete,std::allocator > >` unique name: `.?AU?$default_delete@V?$set@_KU?$less@_K@std@@V?$allocator@_K@2@@std@@@std@@` vtable: , base list: , field list: -- 0x102B | LF_STRUCTURE [size = 88] ``anonymous-namespace'::FileHeader` unique name: `.?AUFileHeader@?A0xa74cdb40@@` vtable: , base list: , field list: -- 0x1031 | LF_STRUCTURE [size = 112] `std::default_delete` unique name: `.?AU?$default_delete@VMemoryBuffer@llvm@@@std@@` vtable: , base list: , field list: -- 0x1081 | LF_STRUCTURE [size = 304] `llvm::AlignedCharArrayUnion >,char,char,char,char,char,char,char,char,char>` unique name: `.?AU?$AlignedCharArrayUnion@V?$unique_ptr@VMemoryBuffer@llvm@@U?$default_delete@VMemoryBuffer@llvm@@@std@@@std@@D@llvm@@` vtable: , base list: , field list: -- 0x1082 | LF_STRUCTURE [size = 176] `llvm::AlignedCharArrayUnion` unique name: `.?AU?$AlignedCharArrayUnion@Verror_code@std@@D@llvm@@` vtable: , base list: , field list: So the interesting thing here is this "unique name" field. This is not possible to access via DIA SDK but it gives us complete rich information about the type that is otherwise impossible. We don't even have to guess, because we can just demangle the name. And coincidentally, I recently just finished writing an Microsoft ABI demangler which is now in LLVM. :) This `.?AU` syntax is non-standard, but it was easy for me to figure out, and I hacked up our demangle library to support this prefix (it's not checked in yet). And basically everything that comes after it exactly matches a mangled type. So, just to give an example. Instead of teaching `CPlusPlusNameParser` to handle ``anonymous namespace'::RawCoverage`, we simply demangle `.?AURawCoverage@?A0xa74cdb40@@`, and we get back a vector of 2 strings which are ``anonymous namespace'` and `RawCoverage`. But instead of just that, there are so many other benefits. Since PDB doesn't contain rich information about template parameters, all we could do until now is just say create an entry in the AST that says "there's a type with this enormously long name that contains angle brackets and other junk". But with this technique, we could actually create legitimate template decls in the AST the way it's supposed to be. There is obviously a lot of complexity in doing it here, but I think long term it will be a richer experience if we parse the mangled name than if we parse the demangled name. But it's only possible with the native plugin. What do you think? https://reviews.llvm.org/D52461 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53951: [NativePDB] Get LLDB types from PDB function types
zturner created this revision. zturner added reviewers: aleksandr.urakov, lemo. This adds basic support for getting function signature types into LLDB's type system, including into clang's AST. There are a few edge cases which are not correctly handled, mostly dealing with nested classes, but this isn't specific to functions and apply equally to variable types. Note that no attempt has been made yet to deal with member function types, which will happen in subsequent patches. https://reviews.llvm.org/D53951 Files: lldb/lit/SymbolFile/NativePDB/Inputs/function-types-builtins.lldbinit lldb/lit/SymbolFile/NativePDB/Inputs/function-types-calling-conv.lldbinit lldb/lit/SymbolFile/NativePDB/Inputs/function-types-classes.lldbinit lldb/lit/SymbolFile/NativePDB/function-types-builtins.cpp lldb/lit/SymbolFile/NativePDB/function-types-calling-conv.cpp lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h llvm/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h Index: llvm/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h === --- llvm/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h +++ llvm/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h @@ -47,7 +47,7 @@ return Error::success(); } template static Expected deserializeAs(CVSymbol Symbol) { -T Record(Symbol.kind()); +T Record(static_cast(Symbol.kind())); if (auto EC = deserializeAs(Symbol, Record)) return std::move(EC); return Record; Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -172,6 +172,8 @@ const llvm::codeview::UnionRecord &ur); lldb::TypeSP CreateArrayType(PdbSymUid type_uid, const llvm::codeview::ArrayRecord &ar); + lldb::TypeSP CreateProcedureType(PdbSymUid type_uid, + const llvm::codeview::ProcedureRecord &pr); lldb::TypeSP CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, clang::TagTypeKind ttk, Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -49,6 +49,8 @@ #include "llvm/Support/ErrorOr.h" #include "llvm/Support/MemoryBuffer.h" +#include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h" + #include "PdbSymUid.h" #include "PdbUtil.h" #include "UdtRecordCompleter.h" @@ -394,6 +396,12 @@ return GetPdbSymType(tpi, LookThroughModifierRecord(cvt)); } +static bool IsCVarArgsFunction(llvm::ArrayRef args) { + if (args.empty()) +return false; + return args.back() == TypeIndex::None(); +} + static clang::TagTypeKind TranslateUdtKind(const TagRecord &cr) { switch (cr.Kind) { case TypeRecordKind::Class: @@ -412,6 +420,32 @@ } } +static llvm::Optional +TranslateCallingConvention(llvm::codeview::CallingConvention conv) { + using CC = llvm::codeview::CallingConvention; + switch (conv) { + + case CC::NearC: + case CC::FarC: +return clang::CallingConv::CC_C; + case CC::NearPascal: + case CC::FarPascal: +return clang::CallingConv::CC_X86Pascal; + case CC::NearFast: + case CC::FarFast: +return clang::CallingConv::CC_X86FastCall; + case CC::NearStdCall: + case CC::FarStdCall: +return clang::CallingConv::CC_X86StdCall; + case CC::ThisCall: +return clang::CallingConv::CC_X86ThisCall; + case CC::NearVector: +return clang::CallingConv::CC_X86VectorCall; + default: +return llvm::None; + } +} + void SymbolFileNativePDB::Initialize() { PluginManager::RegisterPlugin(GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, @@ -540,7 +574,6 @@ PdbSymUid sig_uid = PdbSymUid::makeTypeSymId(PDB_SymType::FunctionSig, TypeIndex{0}, false); Mangled mangled(getSymbolName(sym_record)); - FunctionSP func_sp = std::make_shared( sc.comp_unit, func_uid.toOpaqueId(), sig_uid.toOpaqueId(), mangled, func_type, func_range); @@ -598,6 +631,8 @@ lldb::TypeSP SymbolFileNativePDB::CreatePointerType( PdbSymUid type_uid, const llvm::codeview::PointerRecord &pr) { TypeSP pointee = GetOrCreateType(pr.ReferentType); + if (!pointee) +return nullptr; CompilerType pointee_ct = pointee->GetForwardCompilerType(); lldbassert(pointee_ct); Declaration decl; @@ -670,7 +705,8 @@ return nullptr; ll
[Lldb-commits] [PATCH] D53951: [NativePDB] Get LLDB types from PDB function types
This revision was automatically updated to reflect the committed changes. Closed by commit rL345848: [NativePDB] Get LLDB types from PDB function types. (authored by zturner, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53951?vs=172023&id=172151#toc Repository: rL LLVM https://reviews.llvm.org/D53951 Files: lldb/trunk/lit/SymbolFile/NativePDB/Inputs/function-types-builtins.lldbinit lldb/trunk/lit/SymbolFile/NativePDB/Inputs/function-types-calling-conv.lldbinit lldb/trunk/lit/SymbolFile/NativePDB/Inputs/function-types-classes.lldbinit lldb/trunk/lit/SymbolFile/NativePDB/function-types-builtins.cpp lldb/trunk/lit/SymbolFile/NativePDB/function-types-calling-conv.cpp lldb/trunk/lit/SymbolFile/NativePDB/function-types-classes.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h llvm/trunk/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h Index: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -49,6 +49,8 @@ #include "llvm/Support/ErrorOr.h" #include "llvm/Support/MemoryBuffer.h" +#include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h" + #include "PdbSymUid.h" #include "PdbUtil.h" #include "UdtRecordCompleter.h" @@ -394,6 +396,12 @@ return GetPdbSymType(tpi, LookThroughModifierRecord(cvt)); } +static bool IsCVarArgsFunction(llvm::ArrayRef args) { + if (args.empty()) +return false; + return args.back() == TypeIndex::None(); +} + static clang::TagTypeKind TranslateUdtKind(const TagRecord &cr) { switch (cr.Kind) { case TypeRecordKind::Class: @@ -412,6 +420,32 @@ } } +static llvm::Optional +TranslateCallingConvention(llvm::codeview::CallingConvention conv) { + using CC = llvm::codeview::CallingConvention; + switch (conv) { + + case CC::NearC: + case CC::FarC: +return clang::CallingConv::CC_C; + case CC::NearPascal: + case CC::FarPascal: +return clang::CallingConv::CC_X86Pascal; + case CC::NearFast: + case CC::FarFast: +return clang::CallingConv::CC_X86FastCall; + case CC::NearStdCall: + case CC::FarStdCall: +return clang::CallingConv::CC_X86StdCall; + case CC::ThisCall: +return clang::CallingConv::CC_X86ThisCall; + case CC::NearVector: +return clang::CallingConv::CC_X86VectorCall; + default: +return llvm::None; + } +} + void SymbolFileNativePDB::Initialize() { PluginManager::RegisterPlugin(GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, @@ -540,7 +574,6 @@ PdbSymUid sig_uid = PdbSymUid::makeTypeSymId(PDB_SymType::FunctionSig, TypeIndex{0}, false); Mangled mangled(getSymbolName(sym_record)); - FunctionSP func_sp = std::make_shared( sc.comp_unit, func_uid.toOpaqueId(), sig_uid.toOpaqueId(), mangled, func_type, func_range); @@ -598,6 +631,8 @@ lldb::TypeSP SymbolFileNativePDB::CreatePointerType( PdbSymUid type_uid, const llvm::codeview::PointerRecord &pr) { TypeSP pointee = GetOrCreateType(pr.ReferentType); + if (!pointee) +return nullptr; CompilerType pointee_ct = pointee->GetForwardCompilerType(); lldbassert(pointee_ct); Declaration decl; @@ -639,6 +674,17 @@ } lldb::TypeSP SymbolFileNativePDB::CreateSimpleType(TypeIndex ti) { + if (ti == TypeIndex::NullptrT()) { +PdbSymUid uid = +PdbSymUid::makeTypeSymId(PDB_SymType::BuiltinType, ti, false); +CompilerType ct = m_clang->GetBasicType(eBasicTypeNullPtr); +Declaration decl; +return std::make_shared(uid.toOpaqueId(), this, + ConstString("std::nullptr_t"), 0, nullptr, + LLDB_INVALID_UID, Type::eEncodingIsUID, decl, + ct, Type::eResolveStateFull); + } + if (ti.getSimpleMode() != SimpleTypeMode::Direct) { PdbSymUid uid = PdbSymUid::makeTypeSymId(PDB_SymType::PointerType, ti, false); @@ -670,7 +716,8 @@ return nullptr; lldb::BasicType bt = GetCompilerTypeForSimpleKind(ti.getSimpleKind()); - lldbassert(bt != lldb::eBasicTypeInvalid); + if (bt == lldb::eBasicTypeInvalid) +return nullptr; CompilerType ct = m_clang->GetBasicType(bt); size_t size = GetTypeSizeForSimpleKind(ti.getSimpleKind()); @@ -687,10 +734,6 @@ PdbSymUid type_uid, llvm::StringRef name, size_t size, clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance) { - // Some UDT with trival ctor has zero length. Just ignore. - if (size == 0) -return nullptr; - // Ignore unnamed-tag UDTs. name = DropNameScope(name); if (name.empty()) @@ -792,6 +8
[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32
zturner created this revision. zturner added a reviewer: jingham. char16, char32, and wchar_t were previously broken. If you had a simple variable like `wchar_t x = L'1'` and wrote `p x` LLDB would output `(wchar_t) x = 1\0`. This is because it was using `eFormatChar` with a size of 2. What we needed was to introduce a special format specifically for `wchar_t`. The only valid sizes of `wchar_t` on all existing compilers are 2 and 4, so there's no real point trying to handle sizes of 1 and 8. Along the way, I accidentally stumbled across the reason that references that char16 and char32 types were also formatting incorrectly, so I fixed that as well. https://reviews.llvm.org/D53989 Files: lldb/include/lldb/lldb-enumerations.h lldb/lit/SymbolFile/NativePDB/globals-fundamental.cpp lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Core/DumpDataExtractor.cpp lldb/source/Core/ValueObject.cpp lldb/source/DataFormatters/FormatManager.cpp lldb/source/DataFormatters/VectorType.cpp lldb/source/Symbol/ClangASTContext.cpp Index: lldb/source/Symbol/ClangASTContext.cpp === --- lldb/source/Symbol/ClangASTContext.cpp +++ lldb/source/Symbol/ClangASTContext.cpp @@ -5257,11 +5257,12 @@ return lldb::eFormatBoolean; case clang::BuiltinType::Char_S: case clang::BuiltinType::SChar: -case clang::BuiltinType::WChar_S: case clang::BuiltinType::Char_U: case clang::BuiltinType::UChar: -case clang::BuiltinType::WChar_U: return lldb::eFormatChar; +case clang::BuiltinType::WChar_S: +case clang::BuiltinType::WChar_U: + return lldb::eFormatWchar; case clang::BuiltinType::Char16: return lldb::eFormatUnicode16; case clang::BuiltinType::Char32: Index: lldb/source/DataFormatters/VectorType.cpp === --- lldb/source/DataFormatters/VectorType.cpp +++ lldb/source/DataFormatters/VectorType.cpp @@ -64,9 +64,12 @@ case lldb::eFormatHexFloat: return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloat); + case lldb::eFormatWchar: +return type_system->GetBasicTypeFromAST(lldb::eBasicTypeWChar); case lldb::eFormatUnicode16: +return type_system->GetBasicTypeFromAST(lldb::eBasicTypeChar16); case lldb::eFormatUnicode32: - +return type_system->GetBasicTypeFromAST(lldb::eBasicTypeChar32); case lldb::eFormatUnsigned: return type_system->GetBasicTypeFromAST(lldb::eBasicTypeUnsignedInt); Index: lldb/source/DataFormatters/FormatManager.cpp === --- lldb/source/DataFormatters/FormatManager.cpp +++ lldb/source/DataFormatters/FormatManager.cpp @@ -43,6 +43,7 @@ {eFormatBytesWithASCII, 'Y', "bytes with ASCII"}, {eFormatChar, 'c', "character"}, {eFormatCharPrintable, 'C', "printable character"}, +{eFormatWchar, 'L', "wide character"}, {eFormatComplexFloat, 'F', "complex float"}, {eFormatCString, 's', "c-string"}, {eFormatDecimal, 'd', "decimal"}, Index: lldb/source/Core/ValueObject.cpp === --- lldb/source/Core/ValueObject.cpp +++ lldb/source/Core/ValueObject.cpp @@ -1383,6 +1383,7 @@ (custom_format == eFormatOSType) || (custom_format == eFormatUnicode16) || (custom_format == eFormatUnicode32) || + (custom_format == eFormatWchar) || (custom_format == eFormatUnsigned) || (custom_format == eFormatPointer) || (custom_format == eFormatComplexInteger) || Index: lldb/source/Core/DumpDataExtractor.cpp === --- lldb/source/Core/DumpDataExtractor.cpp +++ lldb/source/Core/DumpDataExtractor.cpp @@ -637,6 +637,23 @@ } } break; +case eFormatWchar: { + s->PutChar('L'); + s->PutChar(item_count == 1 ? '\'' : '\"'); + + const uint64_t ch = DE.GetMaxU64(&offset, item_byte_size); + // wchar_t semantics vary across platforms, and this is complicated even + // more by the fact that the host may not be the same as the target, so to + // be faithful we would have to follow the target's semantics. For + // simplicity, just print the character if it's ascii. + if (ch <= CHAR_MAX && llvm::isPrint(ch)) +s->PutChar(ch); + else +s->PutChar(NON_PRINTABLE_CHAR); + + s->PutChar(item_count == 1 ? '\'' : '\"'); +} break; + case eFormatUnicode16: s->Printf("U+%4.4x", DE.GetU16(&offset)); break; Index: lldb/source/Commands/CommandObjectMemory.cpp === --- lldb/source/Commands/CommandObjectMemory.cpp +++ lldb/source/Commands/CommandObjectMemory.cpp @@ -167,6 +167,15 @@ format_options.GetCountValue() = 8; break; +case eFormatWchar: + if (!byte
[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32
zturner added a comment. No, but thanks for the pointer. Interestingly, it worked for me on my home machine but not on my work machine, and I'm not sure what the difference between the two is. Clearly the test in lang/cpp/wchar_t is making it into `lldb_private::formatters::WCharSummaryProvider` whereas locally I am not. https://reviews.llvm.org/D53989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32
zturner added a comment. Update: It's because There was a problem with my `PYTHONPATH` and python was getting disabled at CMake configure time. So I was effectively running with `LLDB_DISABLE_PYTHON`. So basically it's only broke on the `LLDB_DISABLE_PYTHON` codepath. https://reviews.llvm.org/D53989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53989: Fix formatting of wchar, char16, and char32
zturner added a comment. Side question, should we just kill the `LLDB_DISABLE_PYTHON=0` codepath? It can be a headache to get LLDB linking with Python (particularly on Windows), but it would be nice to be able to delete all the preprocessor stuff. https://reviews.llvm.org/D53989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner created this revision. zturner added reviewers: stella.stamenova, labath, beanz, davide. Herald added subscribers: jfb, delcypher, mgorny, ki.stfu. A year or so ago, I re-wrote most of the lit infrastructure in LLVM so that it wasn't so boilerplate-y. I added lots of common helper type stuff, simplifed usage patterns, and made the code more elegant and maintainable. We migrated to this in LLVM, clang, and lld's lit files, but not in LLDBs. This started to bite me recently, as the 4 most recent times I tried to run the lit test suite in LLDB on a fresh checkout the first thing that would happen is that python would just start crashing with unhelpful backtraces and I would have to spend time investigating. You can reproduce this today by doing a fresh cmake generation, doing `ninja lldb` and then `python bin/llvm-lit.py -sv ~/lldb/lit/SymbolFile` at which point you'll get a segfault that tells you nothing about what your problem is. I started trying to fix the issues with bandaids, but it became clear that the proper solution was to just bring in the work I did in the rest of the projects. The side benefit of this is that the lit configuration files become much cleaner and more understandable as a result. https://reviews.llvm.org/D54009 Files: lldb/lit/Breakpoint/case-insensitive.test lldb/lit/Breakpoint/lit.local.cfg lldb/lit/CMakeLists.txt lldb/lit/Expr/TestIRMemoryMapWindows.test lldb/lit/Expr/lit.local.cfg lldb/lit/Quit/lit.local.cfg lldb/lit/Settings/lit.local.cfg lldb/lit/SymbolFile/NativePDB/lit.local.cfg lldb/lit/SymbolFile/PDB/ast-restore.test lldb/lit/SymbolFile/PDB/calling-conventions.test lldb/lit/SymbolFile/PDB/class-layout.test lldb/lit/SymbolFile/PDB/compilands.test lldb/lit/SymbolFile/PDB/enums-layout.test lldb/lit/SymbolFile/PDB/func-symbols.test lldb/lit/SymbolFile/PDB/function-level-linking.test lldb/lit/SymbolFile/PDB/function-nested-block.test lldb/lit/SymbolFile/PDB/lit.local.cfg lldb/lit/SymbolFile/PDB/pointers.test lldb/lit/SymbolFile/PDB/type-quals.test lldb/lit/SymbolFile/PDB/typedefs.test lldb/lit/SymbolFile/PDB/udt-layout.test lldb/lit/SymbolFile/PDB/variables-locations.test lldb/lit/SymbolFile/PDB/variables.test lldb/lit/Unit/lit.cfg lldb/lit/Unit/lit.cfg.py lldb/lit/Unit/lit.site.cfg.in lldb/lit/Unit/lit.site.cfg.py.in lldb/lit/lit.cfg lldb/lit/lit.cfg.py lldb/lit/lit.site.cfg.in lldb/lit/lit.site.cfg.py.in lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test lldb/lit/tools/lldb-mi/breakpoint/break-insert.test lldb/lit/tools/lldb-mi/data/data-info-line.test lldb/lit/tools/lldb-mi/exec/exec-continue.test lldb/lit/tools/lldb-mi/exec/exec-finish.test lldb/lit/tools/lldb-mi/exec/exec-interrupt.test lldb/lit/tools/lldb-mi/exec/exec-next-instruction.test lldb/lit/tools/lldb-mi/exec/exec-next.test lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test lldb/lit/tools/lldb-mi/exec/exec-step.test lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test llvm/utils/lit/lit/llvm/config.py Index: llvm/utils/lit/lit/llvm/config.py === --- llvm/utils/lit/lit/llvm/config.py +++ llvm/utils/lit/lit/llvm/config.py @@ -55,6 +55,8 @@ features.add('system-windows') elif platform.system() == "Linux": features.add('system-linux') +elif platform.system() in ['FreeBSD']: +config.available_features.add('system-freebsd') # Native compilation: host arch == default triple arch # Both of these values should probably be in every site config (e.g. as Index: lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test === --- lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test +++ lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c %p/inputs/symbol-list-lines.c %p/inputs/list-lines-helper.c -g Index: lldb/lit/tools/lldb-mi/exec/exec-step.test === --- lldb/lit/tools/lldb-mi/exec/exec-step.test +++ lldb/lit/tools/lldb-mi/exec/exec-step.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test === --- lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test +++ lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lldb/lit/tools/lldb-mi/exec/exec-next.test === --- lldb/lit/tools/lldb-mi/exec/exec-next.test +++ lldb/lit/tools/lldb-mi/exec
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner added a comment. In https://reviews.llvm.org/D54009#1284827, @stella.stamenova wrote: > Looks good. The formatting in lit.cfg.py is a bit messy (indentations, > especially), but as long as the tests pass, this is an improvement :). Thanks! Is there something like clang-format for Python? Happy to fix it if so. (I don't do a lot of Python so it's hard for me to eyeball it and figure out what's wrong, so I have to say it looks fine to me :)) https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54031: [NativePDB] Make tests work on x86 too
zturner added a subscriber: aleksandr.urakov. zturner added a comment. Lgtm Repository: rLLDB LLDB https://reviews.llvm.org/D54031 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB346008: Refactor the lit configuration files (authored by zturner, committed by ). Herald added subscribers: teemperor, abidh. Changed prior to commit: https://reviews.llvm.org/D54009?vs=172254&id=172399#toc Repository: rLLDB LLDB https://reviews.llvm.org/D54009 Files: lit/Breakpoint/case-insensitive.test lit/Breakpoint/lit.local.cfg lit/CMakeLists.txt lit/Expr/TestIRMemoryMapWindows.test lit/Expr/lit.local.cfg lit/Quit/lit.local.cfg lit/Settings/lit.local.cfg lit/SymbolFile/NativePDB/lit.local.cfg lit/SymbolFile/PDB/ast-restore.test lit/SymbolFile/PDB/calling-conventions.test lit/SymbolFile/PDB/class-layout.test lit/SymbolFile/PDB/compilands.test lit/SymbolFile/PDB/enums-layout.test lit/SymbolFile/PDB/func-symbols.test lit/SymbolFile/PDB/function-level-linking.test lit/SymbolFile/PDB/function-nested-block.test lit/SymbolFile/PDB/lit.local.cfg lit/SymbolFile/PDB/pointers.test lit/SymbolFile/PDB/type-quals.test lit/SymbolFile/PDB/typedefs.test lit/SymbolFile/PDB/udt-layout.test lit/SymbolFile/PDB/variables-locations.test lit/SymbolFile/PDB/variables.test lit/Unit/lit.cfg lit/Unit/lit.cfg.py lit/Unit/lit.site.cfg.in lit/Unit/lit.site.cfg.py.in lit/lit.cfg lit/lit.cfg.py lit/lit.site.cfg.in lit/lit.site.cfg.py.in lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test lit/tools/lldb-mi/breakpoint/break-insert.test lit/tools/lldb-mi/data/data-info-line.test lit/tools/lldb-mi/exec/exec-continue.test lit/tools/lldb-mi/exec/exec-finish.test lit/tools/lldb-mi/exec/exec-interrupt.test lit/tools/lldb-mi/exec/exec-next-instruction.test lit/tools/lldb-mi/exec/exec-next.test lit/tools/lldb-mi/exec/exec-step-instruction.test lit/tools/lldb-mi/exec/exec-step.test lit/tools/lldb-mi/symbol/symbol-list-lines.test Index: lit/tools/lldb-mi/breakpoint/break-insert.test === --- lit/tools/lldb-mi/breakpoint/break-insert.test +++ lit/tools/lldb-mi/breakpoint/break-insert.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o a.exe %p/inputs/break-insert.c -g Index: lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test === --- lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test +++ lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/break-insert-pending.c -g Index: lit/tools/lldb-mi/data/data-info-line.test === --- lit/tools/lldb-mi/data/data-info-line.test +++ lit/tools/lldb-mi/data/data-info-line.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/data-info-line.c -g Index: lit/tools/lldb-mi/symbol/symbol-list-lines.test === --- lit/tools/lldb-mi/symbol/symbol-list-lines.test +++ lit/tools/lldb-mi/symbol/symbol-list-lines.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c %p/inputs/symbol-list-lines.c %p/inputs/list-lines-helper.c -g Index: lit/tools/lldb-mi/exec/exec-finish.test === --- lit/tools/lldb-mi/exec/exec-finish.test +++ lit/tools/lldb-mi/exec/exec-finish.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lit/tools/lldb-mi/exec/exec-next.test === --- lit/tools/lldb-mi/exec/exec-next.test +++ lit/tools/lldb-mi/exec/exec-next.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lit/tools/lldb-mi/exec/exec-next-instruction.test === --- lit/tools/lldb-mi/exec/exec-next-instruction.test +++ lit/tools/lldb-mi/exec/exec-next-instruction.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lit/tools/lldb-mi/exec/exec-interrupt.test === --- lit/tools/lldb-mi/exec/exec-interrupt.test +++ lit/tools/lldb-mi/exec/exec-interrupt.test @@ -1,4 +1,4 @@ -# XFAIL: windows +# XFAIL: system-windows # -> llvm.org/pr24452 # # RUN: %cc -o %t %p/inputs/main.c -g Index: lit/tools/lldb-mi/exec/exec-step.test === --- lit/tools/lldb-mi/exec/exec-step.test +++ lit/tools/lldb-mi/exec/exec-step.test @@ -1,4 +1,4 @@ -# XFAI
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
zturner added a comment. This function is called in `SymbolFile/NativePDB` as well. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.
zturner created this revision. zturner added reviewers: aleksandr.urakov, labath, lemo. Herald added subscribers: erik.pilkington, mgorny. Previously we were not able to accurately represent tag record types with clang record decls. The primary reason for this is that for type information PDB only contains fully instantiated type names. It does not contain rich information about namespaces, declaration contexts (e.g. parent classes, function scopes for local classes, etc), template parameters, or anything else. All you have is a big long string that is a fully instantiated type name. What it does give you, however, is the mangled name of this type. So we can extract all of the relevant information (minus the distinction between outer class and outer namespace) from the mangled name. This patch is the start of this effort. It uses LLVM's demangler to demangle the name, and treat each component as one component of a namespace chain. Obviously this is not true in the general case, as something like `Foo::Bar(double)::`1'::LocalClass<7>::NestedClass` will get treated as if it were in a series of namespaces. However, it's a good start, and clang doesn't actually care for most uses. So we can improve on this incrementally with subsequent patches to make this more accurate. https://reviews.llvm.org/D54053 Files: lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.cpp lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -175,7 +175,8 @@ lldb::TypeSP CreateProcedureType(PdbSymUid type_uid, const llvm::codeview::ProcedureRecord &pr); lldb::TypeSP - CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, + CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, + llvm::StringRef unique_name, size_t size, clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance); Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -51,6 +51,7 @@ #include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h" +#include "MangledAST.h" #include "PdbSymUid.h" #include "PdbUtil.h" #include "UdtRecordCompleter.h" @@ -731,27 +732,11 @@ } lldb::TypeSP SymbolFileNativePDB::CreateClassStructUnion( -PdbSymUid type_uid, llvm::StringRef name, size_t size, -clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance) { +PdbSymUid type_uid, llvm::StringRef name, llvm::StringRef unique_name, +size_t size, clang::TagTypeKind ttk, +clang::MSInheritanceAttr::Spelling inheritance) { - // Ignore unnamed-tag UDTs. - name = DropNameScope(name); - if (name.empty()) -return nullptr; - - clang::DeclContext *decl_context = m_clang->GetTranslationUnitDecl(); - - lldb::AccessType access = - (ttk == clang::TTK_Class) ? lldb::eAccessPrivate : lldb::eAccessPublic; - - ClangASTMetadata metadata; - metadata.SetUserID(type_uid.toOpaqueId()); - metadata.SetIsDynamicCXXType(false); - - CompilerType ct = - m_clang->CreateRecordType(decl_context, access, name.str().c_str(), ttk, -lldb::eLanguageTypeC_plus_plus, &metadata); - lldbassert(ct.IsValid()); + CompilerType ct = CreateClangDeclFromMangledName(*m_clang, unique_name); clang::CXXRecordDecl *record_decl = m_clang->GetAsCXXRecordDecl(ct.GetOpaqueQualType()); @@ -771,7 +756,7 @@ // FIXME: Search IPI stream for LF_UDT_MOD_SRC_LINE. Declaration decl; return std::make_shared(type_uid.toOpaqueId(), m_clang->GetSymbolFile(), -ConstString(name), size, nullptr, +ct.GetTypeName(), size, nullptr, LLDB_INVALID_UID, Type::eEncodingIsUID, decl, ct, Type::eResolveStateForward); } @@ -782,14 +767,15 @@ clang::MSInheritanceAttr::Spelling inheritance = GetMSInheritance(m_index->tpi().typeCollection(), cr); - return CreateClassStructUnion(type_uid, cr.getName(), cr.getSize(), ttk, -inheritance); + return CreateClassStructUnion(type_uid, cr.getName(), cr.getUniqueName(), +cr.getSiz
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner added a subscriber: stella.stamenova. zturner added a comment. Fix incoming, sorry about that. Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.
zturner added a comment. In https://reviews.llvm.org/D54053#1286272, @lemo wrote: > nice! AFAIU this is just the "default" access of the class. I should probably investigate why it's needed in the first place, since it can be deduced from the tag type. https://reviews.llvm.org/D54053 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.
zturner added a comment. I actually found some issues with this, or at least some potential issues which I'm not sure are actual issues. But I'm adding some new functionality to LLDB to help me confirm whether these are real issues or not, and worst case scenario it will open up some new testing possibilities and help us understand the consequences of messing around with clang ASTs in various ways. So I'm not going to commit this quite yet. Stay tuned. https://reviews.llvm.org/D54053 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54072: Add a command to dump a module's clang ast.
zturner created this revision. zturner added reviewers: vsk, davide, labath, jingham, aleksandr.urakov, clayborg. Herald added subscribers: JDevlieghere, aprantl. This can be useful when diagnosing AST related problems. For example, I had a bug where I was accidentally creating a record type multiple times instead of re-using the first one. This is easy to see with a dump of the AST, because it will look like this: TranslationUnitDecl 0x18a2bc53b98 <> |-CXXRecordDecl 0x18a2bc54458 <> class ClassWithPadding |-CXXRecordDecl 0x18a2bc54520 <> class ClassWithPadding |-CXXRecordDecl 0x18a2bc545e0 <> class ClassWithPadding definition | |-DefinitionData pass_in_registers standard_layout trivially_copyable trivial literal | | |-DefaultConstructor exists trivial needs_implicit | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param | | |-MoveConstructor exists simple trivial needs_implicit | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param | | |-MoveAssignment exists simple trivial needs_implicit | | `-Destructor simple irrelevant trivial needs_implicit | |-MSInheritanceAttr 0x18a2bc546a0 <> Implicit __single_inheritance | |-FieldDecl 0x18a2bc54748 <> a 'char' | |-FieldDecl 0x18a2bc54798 <> b 'short' | |-FieldDecl 0x18a2bc54828 <> c 'char [2]' | |-FieldDecl 0x18a2bc54878 <> d 'int' | |-FieldDecl 0x18a2bc548c8 <> e 'char' | |-FieldDecl 0x18a2bc54918 <> f 'int' | |-FieldDecl 0x18a2bc54968 <> g 'long long' | |-FieldDecl 0x18a2bc549f8 <> h 'char [3]' | |-FieldDecl 0x18a2bc54a48 <> i 'long long' | |-FieldDecl 0x18a2bc54a98 <> j 'char [2]' | |-FieldDecl 0x18a2bc54ae8 <> k 'long long' | |-FieldDecl 0x18a2bc54b38 <> l 'char' | `-FieldDecl 0x18a2bd96f00 <> m 'long long' `- Note there are 3 `CXXRecordDecl`s with the same name, but only one definition. Given the complex interactions between debug info and AST reconstruction, a command like this makes problems within the AST very obvious. I found several other AST-related problems, so this was not even the only one, so I think this is a largely unexplored front when it comes to areas for potentially improved test coverage. And since it's in the REPL, it makes it very easy to test out commands in different orders, get a dump, do something else, get another dump, etc to see how the order of commands affects things. https://reviews.llvm.org/D54072 Files: lldb/include/lldb/Symbol/SymbolFile.h lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -169,6 +169,8 @@ const llvm::pdb::IPDBSession &GetPDBSession() const; + void DumpClangAST() override; + private: struct SecContribInfo { uint32_t Offset; Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1356,6 +1356,14 @@ return types.GetSize(); } +void SymbolFilePDB::DumpClangAST() { + auto type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); + auto clang_type_system = llvm::dyn_cast_or_null(type_system); + if (!clang_type_system) +return; + clang_type_system->getASTContext()->getTranslationUnitDecl()->dumpColor(); +} + void SymbolFilePDB::FindTypesByRegex( const lldb_private::RegularExpression ®ex, uint32_t max_matches, lldb_private::TypeMap &types) { Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -155,6 +155,8 @@ ClangASTContext &GetASTContext() { return *m_clang; } ClangASTImporter &GetASTImporter() { return *m_importer; } + void DumpClangAST() override; + private: size_t FindTypesByName(llvm::StringRef name, uint32_t max_matches, TypeMap &types); Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePD
[Lldb-commits] [PATCH] D54072: Add a command to dump a module's clang ast.
zturner updated this revision to Diff 172510. zturner added a comment. clang dumps to stderr. Only use color if stderr supports this (e.g. if it's not redirected to a file). https://reviews.llvm.org/D54072 Files: lldb/include/lldb/Symbol/SymbolFile.h lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -169,6 +169,8 @@ const llvm::pdb::IPDBSession &GetPDBSession() const; + void DumpClangAST() override; + private: struct SecContribInfo { uint32_t Offset; Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1356,6 +1356,18 @@ return types.GetSize(); } +void SymbolFilePDB::DumpClangAST() { + auto type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); + auto clang = llvm::dyn_cast_or_null(type_system); + if (!clang) +return; + clang::TranslationUnitDecl *tu = clang->getASTContext()->getTranslationUnitDecl(); + if (llvm::errs().has_colors()) +tu->dumpColor(); + else +tu->dump(); +} + void SymbolFilePDB::FindTypesByRegex( const lldb_private::RegularExpression ®ex, uint32_t max_matches, lldb_private::TypeMap &types) { Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -155,6 +155,8 @@ ClangASTContext &GetASTContext() { return *m_clang; } ClangASTImporter &GetASTImporter() { return *m_importer; } + void DumpClangAST() override; + private: size_t FindTypesByName(llvm::StringRef name, uint32_t max_matches, TypeMap &types); Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -1391,6 +1391,17 @@ return 0; } +void SymbolFileNativePDB::DumpClangAST() { + if (!m_clang) +return; + + clang::TranslationUnitDecl *tu = m_clang->getASTContext()->getTranslationUnitDecl(); + if (llvm::errs().has_colors()) +tu->dumpColor(); + else +tu->dump(); +} + uint32_t SymbolFileNativePDB::FindGlobalVariables( const ConstString &name, const CompilerDeclContext *parent_decl_ctx, uint32_t max_matches, VariableList &variables) { Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h === --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -126,6 +126,8 @@ std::vector ParseCallEdgesInFunction(lldb_private::UserID func_id) override; + void DumpClangAST() override; + //-- // PluginInterface protocol //-- Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp === --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -1227,6 +1227,13 @@ return matching_namespace; } +void SymbolFileDWARFDebugMap::DumpClangAST() { + ForEachSymbolFile([](SymbolFileDWARF *oso_dwarf) -> bool { +oso_dwarf->DumpClangAST(); +return true; + }); +} + //-- // PluginInterface protocol //-- Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h === --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -329,6 +329,8 @@ void Dump(lldb_private::Stream &s) override; + void DumpClangAST() override; + protected: typedef llvm::DenseMap
[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.
zturner updated this revision to Diff 172511. zturner added a comment. I just added a new test which dumps the clang AST and makes sure no bogus records get introduced. Since this is already LGTM'ed I'm assuming this is good to go, but since it now depends on https://reviews.llvm.org/D54072, I figure I might as well update this diff since I can't commit it yet. https://reviews.llvm.org/D54053 Files: lldb/lit/SymbolFile/NativePDB/Inputs/ast-reconstruction.lldbinit lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.cpp lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -177,7 +177,8 @@ lldb::TypeSP CreateProcedureType(PdbSymUid type_uid, const llvm::codeview::ProcedureRecord &pr); lldb::TypeSP - CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size, + CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, + llvm::StringRef unique_name, size_t size, clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance); Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp === --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -51,6 +51,7 @@ #include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h" +#include "MangledAST.h" #include "PdbSymUid.h" #include "PdbUtil.h" #include "UdtRecordCompleter.h" @@ -731,27 +732,11 @@ } lldb::TypeSP SymbolFileNativePDB::CreateClassStructUnion( -PdbSymUid type_uid, llvm::StringRef name, size_t size, -clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance) { +PdbSymUid type_uid, llvm::StringRef name, llvm::StringRef unique_name, +size_t size, clang::TagTypeKind ttk, +clang::MSInheritanceAttr::Spelling inheritance) { - // Ignore unnamed-tag UDTs. - name = DropNameScope(name); - if (name.empty()) -return nullptr; - - clang::DeclContext *decl_context = m_clang->GetTranslationUnitDecl(); - - lldb::AccessType access = - (ttk == clang::TTK_Class) ? lldb::eAccessPrivate : lldb::eAccessPublic; - - ClangASTMetadata metadata; - metadata.SetUserID(type_uid.toOpaqueId()); - metadata.SetIsDynamicCXXType(false); - - CompilerType ct = - m_clang->CreateRecordType(decl_context, access, name.str().c_str(), ttk, -lldb::eLanguageTypeC_plus_plus, &metadata); - lldbassert(ct.IsValid()); + CompilerType ct = CreateClangDeclFromMangledName(*m_clang, unique_name); clang::CXXRecordDecl *record_decl = m_clang->GetAsCXXRecordDecl(ct.GetOpaqueQualType()); @@ -771,7 +756,7 @@ // FIXME: Search IPI stream for LF_UDT_MOD_SRC_LINE. Declaration decl; return std::make_shared(type_uid.toOpaqueId(), m_clang->GetSymbolFile(), -ConstString(name), size, nullptr, +ct.GetTypeName(), size, nullptr, LLDB_INVALID_UID, Type::eEncodingIsUID, decl, ct, Type::eResolveStateForward); } @@ -782,14 +767,15 @@ clang::MSInheritanceAttr::Spelling inheritance = GetMSInheritance(m_index->tpi().typeCollection(), cr); - return CreateClassStructUnion(type_uid, cr.getName(), cr.getSize(), ttk, -inheritance); + return CreateClassStructUnion(type_uid, cr.getName(), cr.getUniqueName(), +cr.getSize(), ttk, inheritance); } lldb::TypeSP SymbolFileNativePDB::CreateTagType(PdbSymUid type_uid, const UnionRecord &ur) { return CreateClassStructUnion( - type_uid, ur.getName(), ur.getSize(), clang::TTK_Union, + type_uid, ur.getName(), ur.getUniqueName(), ur.getSize(), + clang::TTK_Union, clang::MSInheritanceAttr::Spelling::Keyword_single_inheritance); } Index: lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h === --- /dev/null +++ lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h @@ -0,0 +1,30 @@ +//===-- MangledAST.h *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is dist
[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.
zturner added a comment. Bleh, I think I found a problem with this approach. Since we assume everything is a namespace, it can lead to ambiguities. I think we need to actually consult the debug info while parsing components of the mangled name. For example, suppose you have this code: namespace NS1 { struct Tag2 { class TagRecord {}; }; } NS1::Tag2::TagRecord ATR; NS1::Tag2 T2; And in LLDB you run the following commands: (lldb) target variable ATR (lldb) target variable T2 We demangle the first name and create an AST from it, but we don't know what `Tag2` is so we assume it's a namespace and we create a `NamespaceDecl`. Then we get to the second one we actually know what it is because we have a precise debug info record for it so we know it's a struct so we create a `CXXRecordDecl` for it. So now we end up creating an invalid AST. Clang will accept it, but it's going to lead to ambiguities on name lookup and create problems down the line. So, I think what we should do instead is ask the debug info "Do you know what the type named `NS1` is in the global scope?" If no, it's a namespace. Otherwise we create (or lookup) the appropriate decl. Then we can repeat this at each step along the way. BTW, when I run those above 2 commands with the patch as it is above, here is the output I get. And you can see that there are two conflicting decls. D:\src\llvmbuild\ninja-x64>"bin\lldb" "-f" "D:\src\llvmbuild\ninja-x64\tools\lldb\lit\SymbolFile\NativePDB\Output\ast-reconstruction.cpp.tmp.exe" (lldb) target create "D:\\src\\llvmbuild\\ninja-x64\\tools\\lldb\\lit\\SymbolFile\\NativePDB\\Output\\ast-reconstruction.cpp.tmp.exe" Current executable set to 'D:\src\llvmbuild\ninja-x64\tools\lldb\lit\SymbolFile\NativePDB\Output\ast-reconstruction.cpp.tmp.exe' (x86_64). (lldb) target variable ATR (NS1::Tag2::TagRecord) ATR = {} (lldb) target variable T2 (NS1::Tag2) T2 = {} (lldb) target modules dump ast TranslationUnitDecl 0x2247dff6fd8 <> |-NamespaceDecl 0x2247dff7898 <> NS1 | |-NamespaceDecl 0x2247dff7918 <> Tag2 | | `-CXXRecordDecl 0x2247dff7998 <> class TagRecord definition | | |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init | | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param | | | |-MoveConstructor exists simple trivial needs_implicit | | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param | | | |-MoveAssignment exists simple trivial needs_implicit | | | `-Destructor simple irrelevant trivial needs_implicit | | `-MSInheritanceAttr 0x2247dff7a60 <> Implicit __single_inheritance | `-CXXRecordDecl 0x2247dff7b08 <> struct Tag2 definition | |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param | | |-MoveConstructor exists simple trivial needs_implicit | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param | | |-MoveAssignment exists simple trivial needs_implicit | | `-Destructor simple irrelevant trivial needs_implicit | `-MSInheritanceAttr 0x2247dff7bd0 <> Implicit __single_inheritance `- Dumping clang ast for 1 modules. https://reviews.llvm.org/D54053 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54072: Add a command to dump a module's clang ast.
zturner added a subscriber: vsk. zturner added a comment. Unfortunately then color output is impossible. Where else would the output be expected to go? https://reviews.llvm.org/D54072 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54072: Add a command to dump a module's clang ast.
zturner added a subscriber: davide. zturner added a comment. Ok so probably this commands output will not go to a log file when logging is enabled? I can’t think of any other side effects. Given that the immediate use case for this is interactive investigation and testing, the first of which is helped by color output and the second of which doesn’t need logging, what do you think of allowing it as is? https://reviews.llvm.org/D54072 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits