[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath updated this revision to Diff 138034. labath added a comment. This drops the LLDBStandalone changes (they are replaced by https://reviews.llvm.org/D44391 on the llvm side), and simplifies the CMAKE_DL_LIBS logic. I've tested this out both with BUILD_SHARED_LIBS and LLVM_LINK_LLVM_DYLIB bu

[Lldb-commits] [PATCH] D40474: DWZ 09/11: Main functionality

labath added a reviewer: davide. labath added a comment. I think the most controversial issue here is the testing... davide may have some thoughts on that. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4529 +SymbolFileDWARF::DWZCommonFile::DWZCommonFile( +

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath added inline comments. Comment at: cmake/modules/LLDBConfig.cmake:349 -if (HAVE_LIBPTHREAD) - list(APPEND system_libs pthread) -endif(HAVE_LIBPTHREAD) - -if (HAVE_LIBDL) - list(APPEND system_libs ${CMAKE_DL_LIBS}) +if(UNIX) + set(CMAKE_THREAD_PREFER_PTHREAD TRUE)

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath planned changes to this revision. labath added a comment. Yes, that thought occurred to me almost immediately after I submitted this. I'll try to implement something like that instead. Since this will be just a bunch of static functions with no state, I think we don't even have to put th

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

labath added inline comments. Comment at: cmake/modules/LLDBConfig.cmake:349 -if (HAVE_LIBPTHREAD) - list(APPEND system_libs pthread) -endif(HAVE_LIBPTHREAD) - -if (HAVE_LIBDL) - list(APPEND system_libs ${CMAKE_DL_LIBS}) +if(UNIX) + set(CMAKE_THREAD_PREFER_PTHREAD TRUE)

[Lldb-commits] [PATCH] D44379: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687)

This revision was automatically updated to reflect the committed changes. Closed by commit rL327490: [cmake] Fix standalone+LLVM_LINK_LLVM_DYLIB builds (pr36687) (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4437

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath updated this revision to Diff 138418. labath added a comment. Herald added a subscriber: mgorny. This is a version that moves the StringTo*** functions to a new "OptionArgParser" class. I'm not terribly proud of the name, but I couldn't think of anything better -- we already have a OptionPa

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. I agree with Jim. I deliberately put here only the types that are used for command parsing, since I presume these are the things that the Command/Interpreter modules will depend on (it's not fully clear to me where to draw the line between these two, so it may end up nee

[Lldb-commits] [PATCH] D44502: Fix a bug in "target.source-map" where we would resolve unmapped paths incorrectly

labath added a comment. I think a .ll file would be better abstraction level for this test. You can still hardcode all the paths needed for the test while avoiding spelling out the irrelevant details like debug info abbreviations. But I guess this will work as well... Repository: rL LLVM h

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath updated this revision to Diff 138540. labath added a comment. s/toAddress/ToAddress/ https://reviews.llvm.org/D44306 Files: include/lldb/Interpreter/Args.h include/lldb/Interpreter/OptionArgParser.h include/lldb/Interpreter/Options.h source/API/SBDebugger.cpp source/Commands/Co

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

labath added a comment. I wonder if we shouldn't just fix the TestDisassembleBreakpoint to not require adding every single architecture. That test was added because lldb-server was not removing the traps from the memory read packets. This is something that is completely independent of us later

[Lldb-commits] [PATCH] D44159: Next batch of test-tree-cleaning changes

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 rL327625: Next batch of test-tree-cleaning changes (authored by labath, committed by ). Herald added a subscriber: llvm-comm

[Lldb-commits] [PATCH] D44526: [dotest] Clean up test folder clean-up

labath created this revision. labath added a reviewer: aprantl. Herald added subscribers: eraman, mgorny, ki.stfu, emaste. This patch implements a unified way of cleaning the build folder of each test. This is done by completely removing the build folder before each test, in the respective setUp()

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I like what you did with the test. Originally, I wanted to just compare the raw memory contents, but this keeps it more inline with the spirit of the original test. I have just one question ab

[Lldb-commits] [PATCH] D44526: [dotest] Clean up test folder clean-up

This revision was automatically updated to reflect the committed changes. Closed by commit rL327703: [dotest] Clean up test folder clean-up (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44526?vs=138579&id=138681#

[Lldb-commits] [PATCH] D44526: [dotest] Clean up test folder clean-up

labath added a subscriber: aprantl. labath added a comment. Deleting the test build dir is fairly easy. I can whip up a patch for that, but I'm not sure if that's the part that is bothering you the most here. Dealing with the log files is a bot more complicated and there doesn't seem to be a clear

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

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 rL327970: Re-land: [lldb] Use vFlash commands when writing to target's flash memory… (authored by labath, committed by ). C

[Lldb-commits] [PATCH] D44680: [dotest] Remove test build directories for passing tests

labath created this revision. labath added reviewers: aprantl, clayborg. This logic is hooked into the same mechanism as the deletion of log files, which makes it possible to use the --log-success argument to have dotest keep the files around (for comparative analysis or whatever). This makes the

[Lldb-commits] [PATCH] D44728: [dotest] Use subprocess.call to forward arguments in wrapper

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thank you. This looks good, assuming that the LLDB_DOTEST_ARGS_STR thingy is working as intended. Comment at: test/lldb-dotest.in:6 dotest_path = '@LLDB_SOURCE_DIR@/test/d

[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen

labath added a comment. The Terminate function is a good place to do this kind of cleanup. Comment at: source/Plugins/Language/D/DLanguage.cpp:34-36 +// D Plugin will define these symbols. They're declared to use with decltype. +extern "C" { +// called once, to initialize drunt

[Lldb-commits] [PATCH] D44738: Add a test for setting the load address of a module with differing physical/virtual addresses

labath created this revision. labath added a reviewer: owenpshaw. First attempt at landing https://reviews.llvm.org/D42145 was reverted because it caused test failures on some android devices. It turned out this was because these devices had vdso modules with differing physical and virtual addres

[Lldb-commits] [PATCH] D44739: [SymbolFileDWARF] Replace FixedFormSizes with llvm::dwarf::getFixedFormByteSize

labath created this revision. labath added reviewers: JDevlieghere, clayborg. The llvm function is practically a drop-in replacement. There may be slight differences in performance as the llvm version uses a switch whereas our version used a lookup table, but I can't say which way that will go. (I

[Lldb-commits] [PATCH] D44739: [SymbolFileDWARF] Replace FixedFormSizes with llvm::dwarf::getFixedFormByteSize

labath added a comment. Hmm... how much is a "significant"? I tried `time optimized/lldb --file debug/clang -o "br set -n dump"` as a benchmark and I saw a 2-3% slowdown (the variance between individual samples is ~1%, so this is statistically significant). I tried playing around with this, an

[Lldb-commits] [PATCH] D44738: Add a test for setting the load address of a module with differing physical/virtual addresses

This revision was automatically updated to reflect the committed changes. Closed by commit rL328485: Add a test for setting the load address of a module with differing… (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D4

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

This revision was automatically updated to reflect the committed changes. Closed by commit rL328488: Add and fix some tests for PPC64 (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44472?vs=138699&id=139777#toc R

[Lldb-commits] [PATCH] D44739: [SymbolFileDWARF] Replace FixedFormSizes with llvm::dwarf::getFixedFormByteSize

labath abandoned this revision. labath added a comment. I'm not sure my benchmark strategy is correct here. I'm getting different (but consistent) results even after making changes that should not impact performance at all (e.g. because I change the code which is not executed). I'll abandon thi

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. So, any objections to this patch? https://reviews.llvm.org/D44306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44922: gdb-remote: Fix checksum verification for messages with escape chars

labath created this revision. labath added reviewers: clayborg, jasonmolenda. Herald added a subscriber: mgorny. We've had a mismatch in the checksum computation between the sender and receiver. The sender computed the payload checksum using the wire encoding of the packet, while the receiver did

[Lldb-commits] [PATCH] D44922: gdb-remote: Fix checksum verification for messages with escape chars

This revision was automatically updated to reflect the committed changes. Closed by commit rL328693: gdb-remote: Fix checksum verification for messages with escape chars (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.o

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

labath added a comment. We already have section dumping code in the lldb-test utility. Extending the output to dump the detected section type seems like a good (and simple) thing to do. Repository: rL LLVM https://reviews.llvm.org/D44998 ___ ll

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

labath added a comment. The patch looks fine to me. I think I'd make the arm test (I guess that is interesting because of the bit 0 twiddling ?) a non-execution test, but this is fine as well. Comment at: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBr

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

labath accepted this revision. labath added a comment. I see, thanks for explaining that. https://reviews.llvm.org/D44998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

labath added a comment. I don't think this is going in a good direction TBH. You are building another layer on top of everything, whereas I think we should be cutting layers out. Besides the issues already pointed out (not being able to differentiate PASS/XFAIL/SKIP, not all .py files being tes

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

labath added a comment. In https://reviews.llvm.org/D45215#1056607, @JDevlieghere wrote: > In https://reviews.llvm.org/D45215#1056043, @labath wrote: > > > I don't think this is going in a good direction TBH. > > > > You are building another layer on top of everything, whereas I think we > > sho

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

labath added a reviewer: zturner. labath added a comment. Adding Zachary as he's familiar with lit internals. I'll try to elaborate more on the approach I had in mind. I would split my approach into a couple of tests. 1. Write a tool which will dump out the list of all tests in the test suite.

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

labath added a comment. In https://reviews.llvm.org/D45215#1056731, @JDevlieghere wrote: > Alright, I'm convinced this is the way to go. > > - For (1) I'll see if I can get some inspiration from the visit logic in > dotest.py. I guess the functionality is similar. I agree on doing this in a > s

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

labath added a comment. > Preferably lit would take care of as much as possible. I think Zachary’s > idea makes sense as an incremental step. If we think of one python file as > a google test executable, it makes sense to return a list of test for every > python file for “v2”. I think running t

[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

labath added a comment. I thing the changes are fine. The only part that worries me is the in-class initialization of the simulator variables. I think this will fail on non-apple hosts. Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py:

[Lldb-commits] [PATCH] D45332: [LIT] Add new LLDB test format

labath added a comment. In https://reviews.llvm.org/D45332#1058976, @JDevlieghere wrote: > In https://reviews.llvm.org/D45332#1058970, @zturner wrote: > > > I don't think `sys.path` is set up correctly to be able to find the > > lldbtest package from the `lldb/lit` folder. > > > > These things k

[Lldb-commits] [PATCH] D45348: Don't return error for settings set .experimental. settings that are absent

labath added inline comments. Comment at: packages/Python/lldbsuite/test/settings/TestSettings.py:544-545 +# the actual name and via .experimental. +cmdinterp.HandleCommand("settings set target.arg0 first-value", result) +self.assertEqual(result.Succeeded(

[Lldb-commits] [PATCH] D45333: WIP: [LIT] Have lit run the lldb test suite

labath added a comment. In https://reviews.llvm.org/D45333#1058916, @JDevlieghere wrote: > This isn't meant to be checked-in as is, however I'm looking for feedback as > early as possible. > > There are currently two problems with the current diff: > > - `./bin/llvm-lit ../llvm/tools/lldb/lit/Su

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. Jim, judging by the comment in https://reviews.llvm.org/D44306#1037587 you were fine with this patch except the naming part. Now that that's fixed, I hope this should be fine. Unless I hear from you, I am going to commit this so I can continue with re-layering the `Args`

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

labath added a comment. Thank you. PS: I will need someone to update the XCode project after this. https://reviews.llvm.org/D44306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44306: Move Args::StringToAddress to Target::EvaluateAddressExpression

This revision was automatically updated to reflect the committed changes. Closed by commit rL329677: Move Args::StringTo*** functions to a new OptionArgParser class (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44306

[Lldb-commits] [PATCH] D45480: Move Args.cpp from Interpreter to Utility

labath created this revision. labath added reviewers: zturner, jingham, davide. Herald added a subscriber: mgorny. The Args class is used in plenty of places besides the command interpreter (e.g., anything requiring an argc+argv combo, such as when launching a process), so it needs to be in a lowe

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

labath added a comment. I don't see myself using this command personally, but I agree that splitting the "disable stats collection" and "dump accumulated stats" into two actions seems a better choice (maybe disable could do a final auto-dump though). I also think the header/footer is not needed

[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first

labath added a comment. The fix seems simple enough, but Jim needs to say whether this is the right way to fix this bug, as I am not sure about what are our assumptions about Breakpoint object lifetimes. Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/b

[Lldb-commits] [PATCH] D45333: WIP: [LIT] Have lit run the lldb test suite

labath accepted this revision. labath added a comment. This looks good. Do you have any plans on how will this work with the XCode build (which is kind of a prerequisite to start removing stuff from dotest)? https://reviews.llvm.org/D45333 ___ ll

[Lldb-commits] [PATCH] D45480: Move Args.cpp from Interpreter to Utility

labath added a comment. In https://reviews.llvm.org/D45480#1063268, @zturner wrote: > > This removes the last (direct) dependency from the Host module to > > Interpreter, so I remove the Interpreter module from Host's dependency list. > > Big milestone! Kudos Thanks. Host is still a member of

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

labath added a comment. In https://reviews.llvm.org/D45547#1065661, @davide wrote: > I prefer having it as a top level command rather than part of log. If you > think about it LLVM does the same distinction and it worked quite well in > practice. > We plan to collect more metrics to the comman

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

labath created this revision. labath added reviewers: jingham, JDevlieghere. If the remote stub sends a specific error message instead of just a E?? code, we can use this to display a more informative error message instead of just the generic "unable to attach" message. I write a test for this us

[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

labath added a comment. Thank you for the patch. For testing I'd recommend taking a look at r320813 https://reviews.llvm.org/D40616, which implemented the SHF_COMPRESSED part of the compressed section support. It looks like we should add a new field to the "lldb-test module-sections" output, so

[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms

labath added a comment. Thanks for the heads up. I can test the android part on Wednesday, but right now I don't see a reason why that shouldn't work there. Overall, I like the idea of using the process class for caching some platform resources. If we end up needing to do that more often, we ca

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

labath added a comment. The part I am not sure about here is that you are creating a Module which has no associated object file, but it still has some sections. That's not how any of our current modules/object files work, and I worry that this may cause problems down the line (and plainly put,

[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms

labath added a comment. I don't think doing this is necessary when we have only one customer, but if we are going to be designing an general purpose storage facility then I don't think we should be using strings (and particularly not ConstStrings) as the lookup keys. I would propose going for

[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms

labath added a comment. I like the idea of leaving some comment to have a record that we discussed making a more generic storage facility, but I don't think we need to do anything else right now. I doubt we will see many new clients for this very soon. Comment at: source/Plu

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

labath added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py:53 +for module in self.target.modules: +self.assertTrue(module.IsValid()) + lemo wrote: > labath wrote: > > Could we

[Lldb-commits] [PATCH] D45703: Use a utility function to speed up "process load image" for POSIX Platforms

labath added inline comments. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166 + // The dlopen succeeded! + if (token != 0x0) +return process->AddImageToken(token); jingham wrote: > labath wrote: > > Use LLDB_INVALID_IMAGE_TOKEN here? > That

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

This revision was automatically updated to reflect the committed changes. labath marked an inline comment as done. Closed by commit rL330247: Report more precise error message when attach fails (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

labath added a comment. It looks like nobody except me is worried about the module-without-an-object-file situation, so I guess we can try this out and see how it goes. The test you've added here has been failing on windows though. I've tried to fix this in r330314, but it meant modifying the

[Lldb-commits] [PATCH] D45949: [dotest] Make the set of tests independent of the test configuration

labath created this revision. labath added reviewers: JDevlieghere, aprantl. Herald added a subscriber: eraman. In the magic test duplicator, we were making the decision whether to create a test variant based on the compiler and the target platform. This meant that the set of known tests was diffe

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

labath added inline comments. Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:130-131 +// we must leave the slash character though. +while (relative_path.consume_front(".")) + /* Do nothing. */; + } What about paths like `.foo/bar.c` an

[Lldb-commits] [PATCH] D45949: [dotest] Make the set of tests independent of the test configuration

This revision was automatically updated to reflect the committed changes. Closed by commit rL330708: [dotest] Make the set of tests independent of the test configuration (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

labath created this revision. labath added reviewers: JDevlieghere, aprantl, amccarth. The utility iterates through the tests in the suite, much like dotest does, and prints all tests it found. So that it can set up the environment properly, it needs to be called with a single argument - the path

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

labath added inline comments. Comment at: source/Utility/FileSpec.cpp:406 + m_directory.Clear(); + } } clayborg wrote: > > they resolve to the same file in a virtual filesystem, where all referenced > > path components exist and are directories > > Norma

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

labath added inline comments. Comment at: source/Utility/FileSpec.cpp:406 + m_directory.Clear(); + } } clayborg wrote: > labath wrote: > > clayborg wrote: > > > > they resolve to the same file in a virtual filesystem, where all > > > > referenced path com

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

labath added a comment. In https://reviews.llvm.org/D46005#1076978, @zturner wrote: > Does it print just the names of the .py files, or does it print the actual > test classes and methods in the Class.TestName format, evaluate XFAIL and > SKIPs, etc? It prints everything: test file (including

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

labath added a comment. In https://reviews.llvm.org/D46005#1077013, @zturner wrote: > I thought the intention was going to be parallelize at file-granularity > rather than method granularity, since the whole point of grouping tests > together into classes is that they can share similar set up a

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

labath added a comment. In https://reviews.llvm.org/D46005#1077016, @zturner wrote: > Note that there's currently no precedent (that i'm aware of anwyay) in LLVM > or any of its subprojects for splitting the running of a single file into > multiple parallel jobs. All of LLVM's other projects p

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

labath added a comment. This code itself looks fine, I have just two minor comments. However, I do have a question about performance. I remember us being very worried about performance in the past, so we ended up putting in this like r298876. This removes the normalization step during FileSpec

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks fine to me. Normalization, at least as it is implemented now in `remove_dots`, is a fairly heavy operation, so it makes sense to avoid it when possible. And the extra speedup is great.

[Lldb-commits] [PATCH] D40468: DWZ 01/07: Support reading section ".gnu_debugaltlink"

labath added a comment. Could you add a "section type" line in the `dumpModules` in lldb-test.cpp. Then we can write a test for this. (not that this is a extremely dangerous change, but it seems a shame not to test it when it is so easy). https://reviews.llvm.org/D40468

[Lldb-commits] [PATCH] D40468: DWZ 01/07: Support reading section ".gnu_debugaltlink"

labath added a comment. Thanks, that's great, but I don't see you using that functionality to test the debug-alt-link classification (I guess I wasn't clear about that, but that is the reason why I asked you to add it). Could you add a test with the new section as well? https://reviews.llvm.o

[Lldb-commits] [PATCH] D46144: Reflow paragraphs in comments.

labath added a comment. The formatting of the comments has been bugging me as well. So far, I have been trying to reflow them whenever I touch the code around them, but at that rate fixing all of them would take ages. Your heuristics seem to be working fairly well. I've found a couple of sub-o

[Lldb-commits] [PATCH] D40468: DWZ 01/07: Support reading section ".gnu_debugaltlink"

labath accepted this revision. labath added a comment. Awesome, thanks. I think we can get this one out of the way. In https://reviews.llvm.org/D40468#1080136, @jankratochvil wrote: > Added `lit/Modules/dwarf-gnu-debugaltlink.yaml`. I do not see its test > success anywhere but when I make it fa

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

labath added a comment. Yes, I take it you need someone to check it in? https://reviews.llvm.org/D44998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46144: Reflow paragraphs in comments.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks fine to me, I guess if noone objects, we can land this. Thanks for doing this. https://reviews.llvm.org/D46144 ___ lldb-commits mailing li

[Lldb-commits] [PATCH] D46128: Fix expression parser to not accept any type whose basename matches for a type that must exist at root level

labath resigned from this revision. labath added a comment. Jim seems to have an idea on how the type lookup should work, so I'll defer to him. https://reviews.llvm.org/D46128 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.l

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

labath added a comment. I've committed this as r331173. I've had to revert the lldb-test changes, as something similar has been implemented already, and I've merged your lldb-test test with an existing section types test. https://reviews.llvm.org/D44998 _

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

This revision was automatically updated to reflect the committed changes. Closed by commit rL331173: ObjectFileELF: Add support for arbitrarily named code sections (authored by labath, committed by ). Herald added a subscriber: JDevlieghere. Changed prior to commit: https://reviews.llvm.org/D44

[Lldb-commits] [PATCH] D40469: DWZ 02/07: Match also DW_TAG_partial_unit when DW_TAG_compile_unit is matched

labath edited reviewers, added: aprantl, JDevlieghere; removed: labath. labath added a comment. Adding some debug info people, as I don't feel qualified to review this. https://reviews.llvm.org/D40469 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex

labath added inline comments. Comment at: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:317 void DWARFUnit::ClearDIEs(bool keep_compile_unit_die) { if (m_die_array.size() > 1) { +llvm::sys::ScopedWriter lock(m_extractdies_mutex); You're accessi

[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules

labath added inline comments. Comment at: include/lldb/Core/Module.h:779 + //-- + void SetUUID(const lldb_private::UUID &uuid); + amccarth wrote: > Was there no SetUUID before because the UUID was

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

labath added inline comments. Comment at: lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py:42 +# Test that static initialized variables can be inspected without process. +self.expect("target variable g_ptr", VARIABLES_DI

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

labath created this revision. labath added reviewers: davide, zturner, asmith, JDevlieghere, clayborg. Herald added subscribers: aprantl, mgorny. Herald added a reviewer: alexshap. lldb-test already had the ability to dump all symbol information in a module. This is interesting, but it can be too

[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules

labath added inline comments. Comment at: include/lldb/Core/Module.h:779 + //-- + void SetUUID(const lldb_private::UUID &uuid); + lemo wrote: > labath wrote: > > amccarth wrote: > > > Was there no

[Lldb-commits] [PATCH] D46334: [CMake] Unify and relayer testing

labath added a comment. Looks fine to me, but I don't use multi-config generators. I'll let you and Stella work out the details there. https://reviews.llvm.org/D46334 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/c

[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules

labath accepted this revision. labath added a comment. I think this is fine. I would have preferred a constructor solution, but the situation there does appear to be a bit messy. I like how you were able to place the SetUUID method next to SetArchitecture. Thanks for your patience. ==

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

labath added inline comments. Comment at: lit/SymbolFile/DWARF/basic-function-lookup.cpp:23 +void foo() {} +// BASE-DAG: name = "foo()", mangled = "_Z3foov" +// FULL-DAG: name = "foo()", mangled = "_Z3foov" zturner wrote: > I personally find it a little confusing

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

labath updated this revision to Diff 144890. labath marked 3 inline comments as done. labath added a comment. Address review comments. https://reviews.llvm.org/D46318 Files: include/lldb/Symbol/VariableList.h lit/CMakeLists.txt lit/SymbolFile/DWARF/find-basic-function.cpp lit/SymbolFile

[Lldb-commits] [PATCH] D46321: Enable AUTOBRIEF in doxygen configuration.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I don't really have an opinion on this, but I don't see why not. https://reviews.llvm.org/D46321 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

labath updated this revision to Diff 144904. labath marked 4 inline comments as done. labath added a comment. More review feedback. https://reviews.llvm.org/D46318 Files: include/lldb/Symbol/VariableList.h lit/CMakeLists.txt lit/SymbolFile/DWARF/find-basic-function.cpp lit/SymbolFile/DW

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

labath added inline comments. Comment at: tools/lldb-test/lldb-test.cpp:260 +if (!RE.IsValid()) { + errs() << "ERROR: `" << Name << "` is not a valid regular expression.\n"; + return; JDevlieghere wrote: > JDevlieghere wrote: > > You can use `WithCo

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

labath added a comment. One day I'd like us to have a unit testing framework for the dwarf expression evaluator. The evaluator looks like a thing that is both complex enough (so it's worth doing it) and /ought to be/ self-contained enough (so it is doable). Then we could test these, and all oth

[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup

This revision was automatically updated to reflect the committed changes. labath marked 2 inline comments as done. Closed by commit rL331447: lldb-test symbols: Add ability to do name-based lookup (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commi

[Lldb-commits] [PATCH] D46334: [CMake] Unify and relayer testing

labath added subscribers: aprantl, davide, JDevlieghere, zturner. labath added a comment. I remember hearing some people use the check-lldb-single target, but it had to do with running the tests against some remote devices/stubs that did not handle multiple connections very well. However, the way

[Lldb-commits] [PATCH] D46395: Remove Process references from the Host module

labath created this revision. labath added reviewers: jingham, davide. Herald added a subscriber: emaste. The Process class was only being referenced because of the last-ditch effort in the process launchers to set a process exit callback in case one isn't set already. Although launching a proces

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

labath added a comment. It looks like this has caused a bunch of tests to crash on linux http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/22800. The crashes seem to be happening during printing of global variables ( `HandleCommand(command = "target variable my_global_char

[Lldb-commits] [PATCH] D46395: Remove Process references from the Host module

labath marked an inline comment as done. labath added a comment. I've added a comment about the requirement to the `MonitoringProcessLauncher` and to `Host::LaunchProcess` (most users go through the latter). I've also tried to make it more obvious that the no-op callback still causes the process

[Lldb-commits] [PATCH] D46395: Remove Process references from the Host module

labath updated this revision to Diff 145163. labath marked an inline comment as done. labath added a comment. Update the diff. https://reviews.llvm.org/D46395 Files: include/lldb/Host/Host.h include/lldb/Host/MonitoringProcessLauncher.h include/lldb/Target/ProcessLaunchInfo.h source/Hos

<    5   6   7   8   9   10   11   12   13   14   >