Re: [Lldb-commits] [PATCH] D24629: Allow for tests to be disabled at runtime
labath added a comment. I don't think this is a totally bad idea. In fact we already had something like this (nobody used it though), before it was removed in https://reviews.llvm.org/rL255040. If it goes in, we might start using it actually -- e.g., currently we have watchpoint tests which fail on some devices which do not support watchpoints. There is no reasonable thing we can base the expectation as the exact same device with a different cpu revision could support watchpoints just fine, so we could just define the list of these tests externally (in this case, I would probably annotate them with the `watchpoint` category and then do the skips based on categories instead). That said, I do have slightly mixed feelings about it, as it is increasing the complexity of an already complex system, and there are other possible ways to solve the watchpoint problem (have the tests detect whether the device supports watchpoints, and self-skip when appropriate). Comment at: packages/Python/lldbsuite/test/dotest.py:803 @@ +802,3 @@ +if configuration.skip_files: +import re +for file_regexp in configuration.skip_files: We should just `import re` at top level. A lot of tests already do that, so it's not likely it will break anyone. https://reviews.llvm.org/D24629 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid added a comment. comments inline. Comment at: packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:43 @@ -42,2 +42,3 @@ """Test to selectively watch different bytes in a 8-byte array.""" -self.run_watchpoint_size_test('byteArray', 8, '1') +if self.getArchitecture() in ['arm']: +self.run_watchpoint_size_test('byteArray', 8, '1', 1) labath wrote: > It's not clear to me why you need to modify the existing test for this > change. You are adding functionality, so all existing tests should pass as-is > (which will also validate that your change did not introduce regressions). As we keep on adding these tests its increasing our overall testing time. I just thought using the same test with some changes will cover the cases we want to test. Anyways we can write separate tests as well. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:556 @@ +555,3 @@ +uint32_t current_size = GetWatchpointSize(wp_index); +if ((current_size == 4) || (current_size == 2 && watch_mask <= 2) || +(current_size == 1 && watch_mask == 1)) labath wrote: > The logic here is extremely convoluted. Doesn't this code basically boil down > to: > ``` > current_size = m_hwp_regs[wp_index].control & 1 ? GetWatchpointSize(wp_index) > : 0; > new_size = llvm::NextPowerOf2(std::max(current_size, watch_mask)); > // update the control value, write the debug registers... > ``` > > Also `watch_mask` should probably be renamed to `watch_size`, as it doesn't > appear to be a mask. Seems legit. I ll update this in next patch. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:602 @@ -612,3 +601,3 @@ bool NativeRegisterContextLinux_arm::ClearHardwareWatchpoint( uint32_t wp_index) { labath wrote: > This looks a bit worrying. > What will happen after the following sequence of events: > - client tells us to set a watchpoint at 0x1000 > - we set the watchpoint > - client tells us to set a watchpoint at 0x1001 > - we extend the previous watchpoint to watch this address as well > - client tells us to delete the watchpoint at 0x1000 > - ??? > > Will we remain watching the address 0x1001? I don't see how will you be able > to do that without maintaining a some info about the original watchpoints the > client requested (and I have not seen that code). Please add a test for this. I just realized that I missed a crucial change in my last patch that we need to make all this work. Let me get back with correction and desired test cases. https://reviews.llvm.org/D24610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels
Author: ldrumm Date: Fri Sep 16 06:28:12 2016 New Revision: 281717 URL: http://llvm.org/viewvc/llvm-project?rev=281717&view=rev Log: [RenderScript] Support tracking and dumping reduction kernels Initial implementation of support for tracking [RenderScript Reductions](https://developer.android.com/guide/topics/renderscript/compute.html#reduction-in-depth) With this patch, `language renderscript module dump` properly lists reductions that are part of loaded RenderScript modules as well the the consituent functions and their roles, within the reduction. This support required new tracking mechanisms for the `#pragma(reduce)` mechanism, and extension of `RSModuleDescriptor::ParseRSInfo` to support the metadata output by `bcc`. This work was also an opportunity to refactor/improve parse code: - `RSModuleDescriptor::ParseExportReduceCount` now has a complete implementation and the debugger can correctly track reductions on receipt of a module hook. - `RSModuleDescriptor::Dump` now dumps Reductions as well as `ForEach` kernels. Also, fixed indentation of the output, and made indentation groupings in the source clearer. - `RSModuleDescriptor::ParseRSInfo` now returns true if the `".rs.info"` packet has nonzero linecount, rather than rejecting RenderScripts that don't contain kernels (an unlikely situation, but possibly valid). This was changed because scripts that only contained reductions were not being tracked in `RenderScriptRuntime::LoadModule`. - Refactor `RSModuleInfo::ParseRSInfo` and add reduction spec parser stub - Prepared ParseRSInfo to more easily be able to add new parser types - Use llvm::StringRef and llvm::StringMap helpers to make the parsing code cleaner - factor out forEachCount, globalVarCount, and pragmaCount parsing block to their own methods - Add ExportReduceCount Parser - Use `llvm::StringRef` in `RSKernelDescriptor` constructor - removed now superfluous `MAXLINE` macros as we've switched from `const char *` to `llvm::StringRef` Modified: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h Modified: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=281717&r1=281716&r2=281717&view=diff == --- lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp (original) +++ lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp Fri Sep 16 06:28:12 2016 @@ -10,6 +10,8 @@ // C Includes // C++ Includes // Other libraries and framework includes +#include "llvm/ADT/StringMap.h" + // Project includes #include "RenderScriptRuntime.h" @@ -2587,17 +2589,104 @@ void RenderScriptRuntime::Update() { } } -// The maximum line length of an .rs.info packet -#define MAXLINE 500 -#define STRINGIFY(x) #x -#define MAXLINESTR_(x) "%" STRINGIFY(x) "s" -#define MAXLINESTR MAXLINESTR_(MAXLINE) +bool RSModuleDescriptor::ParsePragmaCount(llvm::StringRef *lines, + size_t n_lines) { + // Skip the pragma prototype line + ++lines; + for (; n_lines--; ++lines) { +const auto kv_pair = lines->split(" - "); +m_pragmas[kv_pair.first.trim().str()] = kv_pair.second.trim().str(); + } + return true; +} + +bool RSModuleDescriptor::ParseExportReduceCount(llvm::StringRef *lines, +size_t n_lines) { + // The list of reduction kernels in the `.rs.info` symbol is of the form + // "signature - accumulatordatasize - reduction_name - initializer_name - + // accumulator_name - combiner_name - + // outconverter_name - halter_name" + // Where a function is not explicitly named by the user, or is not generated + // by the compiler, it is named "." so the + // dash separated list should always be 8 items long + Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE); + // Skip the exportReduceCount line + ++lines; + for (; n_lines--; ++lines) { +llvm::SmallVector spec; +lines->split(spec, " - "); +if (spec.size() != 8) { + if (spec.size() < 8) { +if (log) + log->Error("Error parsing RenderScript reduction spec. wrong number " + "of fields"); +return false; + } else if (log) +log->Warning("Extraneous members in reduction spec: '%s'", + lines->str().c_str()); +} + +const auto sig_s = spec[0]; +uint32_t sig; +if (sig_s.getAsInteger(10, sig)) { + if (log) +log->Error("Error parsing Renderscript reduction spec: invalid kernel " + "signature: '%s'", +
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
omjavaid updated this revision to Diff 71633. omjavaid added a comment. Herald added subscribers: srhines, danalbert, tberghammer. I have added a new test case that tests suggested scnario without changing any previous test cases. Also I have made sure we re validate all watchpoint installed on thread resume to make sure we have the latest values assigned to hardware watchpoint registers. This passes on ARM (RaspberryPi3, Samsung Chromebook). I have not yet tested on android. This will fail on targets which dont support multiple watchpoint slots. Also this should fail on AArch64 which I am currently working on. https://reviews.llvm.org/D24610 Files: packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp source/Plugins/Process/Linux/NativeThreadLinux.cpp Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp === --- source/Plugins/Process/Linux/NativeThreadLinux.cpp +++ source/Plugins/Process/Linux/NativeThreadLinux.cpp @@ -198,6 +198,9 @@ m_stop_info.reason = StopReason::eStopReasonNone; m_stop_description.clear(); + // Invalidate watchpoint index map to re-sync watchpoint registers. + m_watchpoint_index_map.clear(); + // If watchpoints have been set, but none on this thread, // then this is a new thread. So set all existing watchpoints. if (m_watchpoint_index_map.empty()) { Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp === --- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp +++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp @@ -410,15 +410,13 @@ if ((m_hbr_regs[bp_index].control & 1) == 0) { m_hbr_regs[bp_index].address = addr; m_hbr_regs[bp_index].control = control_value; -m_hbr_regs[bp_index].refcount = 1; // PTRACE call to set corresponding hardware breakpoint register. error = WriteHardwareDebugRegs(eDREGTypeBREAK, bp_index); if (error.Fail()) { m_hbr_regs[bp_index].address = 0; m_hbr_regs[bp_index].control &= ~1; - m_hbr_regs[bp_index].refcount = 0; return LLDB_INVALID_INDEX32; } @@ -508,8 +506,19 @@ if (error.Fail()) return LLDB_INVALID_INDEX32; - uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0; + uint32_t control_value = 0; lldb::addr_t real_addr = addr; + uint32_t wp_index = LLDB_INVALID_INDEX32; + + // Find out how many bytes we need to watch after 4-byte alignment boundary. + uint8_t watch_size = (addr & 0x03) + size; + + // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary. + if (size == 0 || watch_size > 4) +return LLDB_INVALID_INDEX32; + + // Strip away last two bits of address for byte/half-word/word selection. + addr &= ~((lldb::addr_t)3); // Check if we are setting watchpoint other than read/write/access // Also update watchpoint flag to match Arm write-read bit configuration. @@ -526,86 +535,58 @@ return LLDB_INVALID_INDEX32; } - // Can't watch zero bytes - // Can't watch more than 4 bytes per WVR/WCR pair - - if (size == 0 || size > 4) -return LLDB_INVALID_INDEX32; - - // Check 4-byte alignment for hardware watchpoint target address. - // Below is a hack to recalculate address and size in order to - // make sure we can watch non 4-byte alligned addresses as well. - if (addr & 0x03) { -uint8_t watch_mask = (addr & 0x03) + size; - -if (watch_mask > 0x04) - return LLDB_INVALID_INDEX32; -else if (watch_mask <= 0x02) - size = 2; -else if (watch_mask <= 0x04) - size = 4; - -addr = addr & (~0x03); + // Iterate over stored watchpoints and find a free or duplicate wp_index + for (uint32_t i = 0; i < m_max_hwp_supported; i++) { +if ((m_hwp_regs[i].control & 1) == 0) { + wp_index = i; // Mark last free slot +} else if (m_hwp_regs[i].address == addr) { + wp_index = i; // Mark duplicate index + break;// Stop searching here +} } - // We can only watch up to four bytes that follow a 4 byte aligned address - // per watchpoint register pair, so make sure we can properly encode this. - addr_word_offset = addr % 4; - byte_mask = ((1u << size) - 1u) << addr_word_offset; - - // Check if we need multiple watchpoint register - if (byte_mask > 0xfu) + // No vaccant slot available and no duplicate slot found. + if (wp_index == LLDB_INVALID_INDEX32) return LLDB_INVALID_INDEX32; + uint8_t current_watch_size, new_watch_size; + // Calculate overall size width to be watched by current hardware watchpoint slot. + current_watch_
Re: [Lldb-commits] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels
On Fri, Sep 16, 2016 at 4:36 AM Luke Drummond via lldb-commits < lldb-commits@lists.llvm.org> wrote: > Author: ldrumm > Date: Fri Sep 16 06:28:12 2016 > New Revision: 281717 > > URL: http://llvm.org/viewvc/llvm-project?rev=281717&view=rev > Log: > [RenderScript] Support tracking and dumping reduction kernels > > Initial implementation of support for tracking > [RenderScript Reductions]( > https://developer.android.com/guide/topics/renderscript/compute.html#reduction-in-depth > ) > > With this patch, `language renderscript module dump` properly lists > reductions > that are part of loaded RenderScript modules as well the the consituent > functions and their roles, within the reduction. > > This support required new tracking mechanisms for the `#pragma(reduce)` > mechanism, and extension of `RSModuleDescriptor::ParseRSInfo` to support > the metadata output by `bcc`. This work was also an opportunity to > refactor/improve parse code: > > - `RSModuleDescriptor::ParseExportReduceCount` now has a complete > implementation and the debugger can correctly track reductions on > receipt of a module hook. > - `RSModuleDescriptor::Dump` now dumps Reductions as well as `ForEach` > kernels. Also, fixed indentation of the output, and made indentation > groupings in the source clearer. > - `RSModuleDescriptor::ParseRSInfo` now returns true if the `".rs.info"` > packet has nonzero linecount, rather than rejecting RenderScripts that > don't contain kernels (an unlikely situation, but possibly valid). This > was changed because scripts that only contained reductions were not > being tracked in `RenderScriptRuntime::LoadModule`. > - Refactor `RSModuleInfo::ParseRSInfo` and add reduction spec parser stub > - Prepared ParseRSInfo to more easily be able to add new parser types > - Use llvm::StringRef and llvm::StringMap helpers to make the parsing > code cleaner > - factor out forEachCount, globalVarCount, and pragmaCount parsing block > to their own methods > - Add ExportReduceCount Parser > - Use `llvm::StringRef` in `RSKernelDescriptor` constructor > - removed now superfluous `MAXLINE` macros as we've switched from `const >char *` to `llvm::StringRef` > > Modified: > > lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp > > lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h > > Modified: > lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=281717&r1=281716&r2=281717&view=diff > > == > --- > lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp > (original) > +++ > lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp > Fri Sep 16 06:28:12 2016 > @@ -10,6 +10,8 @@ > // C Includes > // C++ Includes > // Other libraries and framework includes > +#include "llvm/ADT/StringMap.h" > + > // Project includes > #include "RenderScriptRuntime.h" > > @@ -2587,17 +2589,104 @@ void RenderScriptRuntime::Update() { >} > } > > -// The maximum line length of an .rs.info packet > -#define MAXLINE 500 > -#define STRINGIFY(x) #x > -#define MAXLINESTR_(x) "%" STRINGIFY(x) "s" > -#define MAXLINESTR MAXLINESTR_(MAXLINE) > +bool RSModuleDescriptor::ParsePragmaCount(llvm::StringRef *lines, > + size_t n_lines) { > + // Skip the pragma prototype line > + ++lines; > + for (; n_lines--; ++lines) { > +const auto kv_pair = lines->split(" - "); > +m_pragmas[kv_pair.first.trim().str()] = kv_pair.second.trim().str(); > + } > + return true; > +} > + > +bool RSModuleDescriptor::ParseExportReduceCount(llvm::StringRef *lines, > +size_t n_lines) { > This should take an ArrayRef. In general this is true any time you're passing an array to a function via (T* items, size_t length). > + // The list of reduction kernels in the `.rs.info` symbol is of the > form > + // "signature - accumulatordatasize - reduction_name - initializer_name > - > + // accumulator_name - combiner_name - > + // outconverter_name - halter_name" > + // Where a function is not explicitly named by the user, or is not > generated > + // by the compiler, it is named "." so the > + // dash separated list should always be 8 items long > + Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE); > + // Skip the exportReduceCount line > + ++lines; > + for (; n_lines--; ++lines) { > With ArrayRef this would be written as: for (StringRef line : lines.drop_front()) { } > +llvm::SmallVector spec; > +lines->split(spec, " - "); > +if (spec.size() != 8) { > + if (spec.size() < 8) { > +i
Re: [Lldb-commits] [PATCH] D24629: Allow for tests to be disabled at runtime
fjricci updated this revision to Diff 71651. fjricci added a comment. Refactor re https://reviews.llvm.org/D24629 Files: packages/Python/lldbsuite/test/configuration.py packages/Python/lldbsuite/test/dotest.py packages/Python/lldbsuite/test/dotest_args.py packages/Python/lldbsuite/test/test_result.py Index: packages/Python/lldbsuite/test/test_result.py === --- packages/Python/lldbsuite/test/test_result.py +++ packages/Python/lldbsuite/test/test_result.py @@ -18,6 +18,8 @@ # Third-party modules import unittest2 +from unittest2.util import strclass + # LLDB Modules from . import configuration from lldbsuite.test_event.event_builder import EventBuilder @@ -124,10 +126,23 @@ test, test._testMethodName).__func__.__unittest_skip_why__ = "test case does not fall in any category of interest for this run" +def checkExclusion(self, exclusion_list, name): +if exclusion_list: +import re +for item in exclusion_list: +if re.search(item, name): +return True +return False + def startTest(self, test): if configuration.shouldSkipBecauseOfCategories( self.getCategoriesForTest(test)): self.hardMarkAsSkipped(test) +if self.checkExclusion( +configuration.skip_methods, +test._testMethodName): +self.hardMarkAsSkipped(test) + configuration.setCrashInfoHook( "%s at %s" % (str(test), inspect.getfile( @@ -145,6 +160,15 @@ EventBuilder.event_for_start(test)) def addSuccess(self, test): +if self.checkExclusion( +configuration.xfail_files, +strclass( +test.__class__)) or self.checkExclusion( +configuration.xfail_methods, +test._testMethodName): +self.addUnexpectedSuccess(test, None) +return + super(LLDBTestResult, self).addSuccess(test) if configuration.parsable: self.stream.write( @@ -214,6 +238,15 @@ test, err)) def addFailure(self, test, err): +if self.checkExclusion( +configuration.xfail_files, +strclass( +test.__class__)) or self.checkExclusion( +configuration.xfail_methods, +test._testMethodName): +self.addExpectedFailure(test, err, None) +return + configuration.sdir_has_content = True super(LLDBTestResult, self).addFailure(test, err) method = getattr(test, "markFailure", None) Index: packages/Python/lldbsuite/test/dotest_args.py === --- packages/Python/lldbsuite/test/dotest_args.py +++ packages/Python/lldbsuite/test/dotest_args.py @@ -96,6 +96,9 @@ '-p', metavar='pattern', help='Specify a regexp filename pattern for inclusion in the test suite') +group.add_argument('--excluded', metavar='exclusion-file', help=textwrap.dedent( +'''Specify a file for tests to exclude. File should contain lists of regular expressions for test files or methods, +with each list under a matching header (xfail files, xfail methods, skip files, skip methods)''')) group.add_argument( '-G', '--category', Index: packages/Python/lldbsuite/test/dotest.py === --- packages/Python/lldbsuite/test/dotest.py +++ packages/Python/lldbsuite/test/dotest.py @@ -26,6 +26,7 @@ import os import errno import platform +import re import signal import socket import subprocess @@ -202,6 +203,48 @@ sys.exit(0) +def parseExclusion(exclusion_file): +"""Parse an exclusion file, of the following format, where + 'skip files', 'skip methods', 'xfail files', and 'xfail methods' + are the possible list heading values: + + skip files + + + + xfail methods + +""" +excl_type = None +case_type = None + +with open(exclusion_file) as f: +for line in f: +if not excl_type: +[excl_type, case_type] = line.split() +continue + +line = line.strip() +if not line: +excl_type = None +elif excl_type == 'skip' and case_type == 'files': +if not configuration.skip_files: +configuration.skip_files = [] +configuration.skip_files.append(line) +elif excl_type == 'skip' and case_type == 'methods': +if not configuration.skip_methods: +configuration.skip_methods = [] +configuration.skip_methods.append(line) +elif excl_type == 'xfail' and
Re: [Lldb-commits] [PATCH] D24629: Allow for tests to be disabled at runtime
fjricci added a comment. I do understand the complexity problem, and it was one of my concerns with this as well. For my cases, the complexity here is significantly less than the alternatives, but I also do understand if you don't think that's generally true. It probably comes down to how often we think that people are running the test suite in cases where this sort of functionality would be useful. I don't really have a good sense for how other people tend to use the test suite, so I'm personally not sure. For our case, it's a big deal, but if we're the only people who this patch helps, I know it doesn't make sense to merge it. https://reviews.llvm.org/D24629 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows
wallace added a comment. ping https://reviews.llvm.org/D24283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available
wallace added a comment. ping https://reviews.llvm.org/D24284 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows
zturner added a comment. Thanks for the reminder. Will do this today. https://reviews.llvm.org/D24283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available
zturner accepted this revision. This revision is now accepted and ready to land. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:89 @@ +88,3 @@ + if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp)) { +return NULL; + } s/NULL/nullptr/ https://reviews.llvm.org/D24284 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available
wallace added a comment. I'm obliged, your grace https://reviews.llvm.org/D24284 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible
zturner added a subscriber: zturner. Comment at: packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c:23 @@ +22,3 @@ +{ +printf("About to write byteArray[%d] ...\n", i); // About to write byteArray + What's up with all the double spaced source code? Is this intentional? Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:513-521 @@ -513,1 +512,11 @@ + + // Find out how many bytes we need to watch after 4-byte alignment boundary. + uint8_t watch_size = (addr & 0x03) + size; + + // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary. + if (size == 0 || watch_size > 4) +return LLDB_INVALID_INDEX32; + + // Strip away last two bits of address for byte/half-word/word selection. + addr &= ~((lldb::addr_t)3); This block of code is a bit confusing to me. Is this equivalent to: ``` lldb::addr_t start = llvm::alignDown(addr, 4); lldb::addr_t end = addr + size; if (start == end || (end-start)>4) return LLDB_INVALID_INDEX32; ``` https://reviews.llvm.org/D24610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281765 - Set the correct triple when creating an ArchSpec for Windows.
Author: zturner Date: Fri Sep 16 14:09:19 2016 New Revision: 281765 URL: http://llvm.org/viewvc/llvm-project?rev=281765&view=rev Log: Set the correct triple when creating an ArchSpec for Windows. Patch by Walter Erquinigo Differential Revision: https://reviews.llvm.org/D24283 Modified: lldb/trunk/source/Core/ArchSpec.cpp Modified: lldb/trunk/source/Core/ArchSpec.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ArchSpec.cpp?rev=281765&r1=281764&r2=281765&view=diff == --- lldb/trunk/source/Core/ArchSpec.cpp (original) +++ lldb/trunk/source/Core/ArchSpec.cpp Fri Sep 16 14:09:19 2016 @@ -1030,6 +1030,9 @@ bool ArchSpec::SetArchitecture(Architect m_triple.setOS(llvm::Triple::OSType::Solaris); break; } +} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) { + m_triple.setVendor(llvm::Triple::PC); + m_triple.setOS(llvm::Triple::Win32); } else { m_triple.setVendor(llvm::Triple::UnknownVendor); m_triple.setOS(llvm::Triple::UnknownOS); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281764 - Add unit tests for a few string conversion functions in Args.
Author: zturner Date: Fri Sep 16 14:09:12 2016 New Revision: 281764 URL: http://llvm.org/viewvc/llvm-project?rev=281764&view=rev Log: Add unit tests for a few string conversion functions in Args. Also provided a StringRef overload for these functions and have the const char* overloads delegate to the StringRef overload. Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h lldb/trunk/include/lldb/Host/FileSpec.h lldb/trunk/include/lldb/Interpreter/Args.h lldb/trunk/include/lldb/Interpreter/Options.h lldb/trunk/include/lldb/Target/Language.h lldb/trunk/source/API/SBDebugger.cpp lldb/trunk/source/Interpreter/Args.cpp lldb/trunk/source/Interpreter/OptionValueChar.cpp lldb/trunk/source/Interpreter/Property.cpp lldb/trunk/unittests/Interpreter/TestArgs.cpp Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointID.h?rev=281764&r1=281763&r2=281764&view=diff == --- lldb/trunk/include/lldb/Breakpoint/BreakpointID.h (original) +++ lldb/trunk/include/lldb/Breakpoint/BreakpointID.h Fri Sep 16 14:09:12 2016 @@ -87,6 +87,7 @@ public: /// \b true if the name is a breakpoint name (as opposed to an ID or /// range) false otherwise. //-- + // TODO: Convert this function to use a StringRef. static bool StringIsBreakpointName(const char *name, Error &error); //-- Modified: lldb/trunk/include/lldb/Host/FileSpec.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=281764&r1=281763&r2=281764&view=diff == --- lldb/trunk/include/lldb/Host/FileSpec.h (original) +++ lldb/trunk/include/lldb/Host/FileSpec.h Fri Sep 16 14:09:12 2016 @@ -80,6 +80,7 @@ public: /// /// @see FileSpec::SetFile (const char *path, bool resolve) //-- + // TODO: Convert this constructor to use a StringRef. explicit FileSpec(const char *path, bool resolve_path, PathSyntax syntax = ePathSyntaxHostNative); Modified: lldb/trunk/include/lldb/Interpreter/Args.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=281764&r1=281763&r2=281764&view=diff == --- lldb/trunk/include/lldb/Interpreter/Args.h (original) +++ lldb/trunk/include/lldb/Interpreter/Args.h Fri Sep 16 14:09:12 2016 @@ -179,6 +179,7 @@ public: /// @return /// The NULL terminated C string of the copy of \a arg_cstr. //-- + // TODO: Convert this function to use a StringRef. const char *AppendArgument(const char *arg_cstr, char quote_char = '\0'); void AppendArguments(const Args &rhs); @@ -353,24 +354,26 @@ public: return min <= sval64 && sval64 <= max; } + // TODO: Make this function take a StringRef static lldb::addr_t StringToAddress(const ExecutionContext *exe_ctx, const char *s, lldb::addr_t fail_value, Error *error); static bool StringToBoolean(const char *s, bool fail_value, - bool *success_ptr); +bool *success_ptr); static bool StringToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr); - static char StringToChar(const char *s, char fail_value, bool *success_ptr); + static char StringToChar(llvm::StringRef s, char fail_value, + bool *success_ptr); static int64_t StringToOptionEnum(const char *s, OptionEnumValueElement *enum_values, int32_t fail_value, Error &error); static lldb::ScriptLanguage - StringToScriptLanguage(const char *s, lldb::ScriptLanguage fail_value, + StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value, bool *success_ptr); // TODO: Use StringRef Modified: lldb/trunk/include/lldb/Interpreter/Options.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Options.h?rev=281764&r1=281763&r2=281764&view=diff == --- lldb/trunk/include/lldb/Interpreter/Options.h (original) +++ lldb/trunk/include/lldb/Interpreter/Options.h Fri Sep 16 14:09:12 2016 @@ -188,6 +188,7 @@ public: /// @see Args::ParseOptions (Options&) /// @see man getopt_long_only //-- + // TODO: Make this function take a StringRef. virtual Error
Re: [Lldb-commits] [PATCH] D24283: [lldb] Set the correct triple when creating an ArchSpec for windows
This revision was automatically updated to reflect the committed changes. Closed by commit rL281765: Set the correct triple when creating an ArchSpec for Windows. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D24283?vs=71226&id=71686#toc Repository: rL LLVM https://reviews.llvm.org/D24283 Files: lldb/trunk/source/Core/ArchSpec.cpp Index: lldb/trunk/source/Core/ArchSpec.cpp === --- lldb/trunk/source/Core/ArchSpec.cpp +++ lldb/trunk/source/Core/ArchSpec.cpp @@ -1030,6 +1030,9 @@ m_triple.setOS(llvm::Triple::OSType::Solaris); break; } +} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) { + m_triple.setVendor(llvm::Triple::PC); + m_triple.setOS(llvm::Triple::Win32); } else { m_triple.setVendor(llvm::Triple::UnknownVendor); m_triple.setOS(llvm::Triple::UnknownOS); Index: lldb/trunk/source/Core/ArchSpec.cpp === --- lldb/trunk/source/Core/ArchSpec.cpp +++ lldb/trunk/source/Core/ArchSpec.cpp @@ -1030,6 +1030,9 @@ m_triple.setOS(llvm::Triple::OSType::Solaris); break; } +} else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) { + m_triple.setVendor(llvm::Triple::PC); + m_triple.setOS(llvm::Triple::Win32); } else { m_triple.setVendor(llvm::Triple::UnknownVendor); m_triple.setOS(llvm::Triple::UnknownOS); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281770 - Fix compiler warnings where two values weren't being initialized.
Author: gclayton Date: Fri Sep 16 15:10:02 2016 New Revision: 281770 URL: http://llvm.org/viewvc/llvm-project?rev=281770&view=rev Log: Fix compiler warnings where two values weren't being initialized. Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h?rev=281770&r1=281769&r2=281770&view=diff == --- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h (original) +++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h Fri Sep 16 15:10:02 2016 @@ -131,7 +131,7 @@ LLVM_EXTENSION BNDR_OFFSET(i), eEncodingVector, eFormatVectorOfUInt64, \ {dwarf_##reg##i##_i386, dwarf_##reg##i##_i386, LLDB_INVALID_REGNUM, \ LLDB_INVALID_REGNUM, lldb_##reg##i##_i386 }, \ - NULL, NULL \ + NULL, NULL, NULL, 0 \ } #define DEFINE_BNDC(name, i) \ @@ -141,7 +141,7 @@ eFormatVectorOfUInt8, \ {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, \ LLDB_INVALID_REGNUM, lldb_##name##_i386 }, \ -NULL, NULL \ +NULL, NULL, NULL, 0 \ } #define DEFINE_DR(reg, i) \ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] LLVM buildmaster will be updated and restarted tonight
Hello everyone, LLVM buildmaster will be updated and restarted after 6 PM Pacific time today. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I thought about this more overnight and I'm more convinced that lit and lldb-mi make a great pair. lldb-mi is a programmatic text format that isn't subject to the whims of command-line UI design over the years; well tests written in terms of MI will be resilient and stable. lldb-mi MUCH more closely matches a non-lldb developer's view of how a debugger works. The syntax is different from what they use, but it isn't hard to figure out. I haven't touched MI in four or five years and it only took me a couple minutes to figure out how to do run to a breakpoint and print a variable: -file-exec-and-symbols "/tmp/a.out" -break-insert a.c:10 =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x00010f70",func="main",file="a.c",fullname="/tmp/a.c",line="10",times="0",original-location="a.c:10"} -exec-run -thread-info -exec-next -var-create var1 * c ^done,name="var1",numchild="0",value="12",type="int",thread-id="1",has_more="0" (I omitted the responses from all the command except -break-insert and -var-create to make it easier to read, but go run lldb-mi and play with it yourself or read the gdb MI documentation on the web. It's a really simple format to work in.) Several of the proposal have been special commands in the lldb command line driver that have non-changing output styles or the like. Why re-invent something that already exists? This makes a ton more sense on every front. Yes, it means you can't copy and paste your lldb debug session into a test case --- but we've all agreed that that's not something we want anyone doing anyway. I'm in favor of lit + lldb-mi and think this would add a lot of value as an additional way for people to write test cases. J > On Sep 15, 2016, at 7:40 PM, Zachary Turner wrote: > > One thing I wonder about. It seems like everyone is mostly on the same page > about command line output . > > What about input? Would anyone have objections to a test which ran a few > commands to get the debugger into a particular state before doing something > to verify the output? Let's assume I'm waving my hands about this last step > but that it doesn't involve scraping the output of a debugger command > > Maybe this will never even be an issue, but I just want to make sure everyone > is on the same page and that the objections are: > > a) specific to command OUTPUT, not input (i.e. it's ok to have a test run > "break set -n main") > b) specific to *non trivial* output (checking that "p 5" displays 5 is ok) > c) specific to the the output of the *user* command line interface, so that > some hypothetical other command line interface (which again I'm being hand > wavy about) would not be subject to the same objections. > > > On Thu, Sep 15, 2016 at 7:28 PM Jason Molenda wrote: > > > On Sep 15, 2016, at 8:02 AM, Zachary Turner wrote: > > > > > > It sounds like your goal is also "tests have to use the SB API and no other > > API", which if so I think that's counterproductive. More productive, IMO, > > would be being open to any alternative that addresses the concerns you have > > with command-line tests. There are more than 2 ways to skin a cat, so to > > speak. > > Thinking about this a bit, another approach would be to do lit tests on top > of lldb-mi. MI is a structured format for the debugger and a UI to > communicate back and forth with a simple text markup language (it would be > JSON if it were being done today, but it was added to gdb eighteen years ago, > so it's not). The commands correspond to the commands a debugger user would > think to use -- no need to understand the structure of how lldb is > implemented, like with the SB API. The format is a little unusual for a > human to type, but back when we supported gdb at Apple we worked in MI all > the time (it was used to communicate between Xcode, our IDE, and gdb) by hand > when testing and it was fine. "-exec-run" instead of run, that kind of thing. > I think there are four dozens different commands. > > lldb-mi itself uses SB API. And the concerns about hardcoding command line > UI don't apply, it's a key-value format intended for computers, no one is > going to add an extra space character to anything -- the most it changes is > that new key-value pairs are added to responses. > > > I agree there are many acceptable ways to write lit tests that don't involve > lldb command line scraping, and I'm all in favor of using lit with tests that > don't do that. Of course the patch we're discussing has lit test examples > that contradict our own policy on writing tests. Once lit is supported in > lldb, are we going to reject any testsuite additions that depend on the text > output from the command line lldb? If we're all on the same page here, then > I have no reservations. > > Just to say out loud the future I can easily see: We add lit, then we have > people helpfully write a few dozen tests in lit that dep
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I have been thinking about something similar. Ildb-mi specifically I have serious concerns about because the code itself is a bit of an abomination. It has its own entire test suite, which also doesn't work very well. Using a tool like lldb mi is very similar in spirit to how llvm already uses lit though. There is a collection of very specific tools that exist just for testing, and the output of those is run through lit) Lldb-mi was developed independently, had no review, and was essentially a large code drop. and it also was not developed with testing in mind. I agree we could probably make it work, but I've been in that code a few times and I have to say, I don't want to go back. In any case, i think we agree in principle that from a high level, we could get a lot of benefits from this style of test. I'll have more specifics on my thoughts in a few weeks On Fri, Sep 16, 2016 at 4:07 PM Jason Molenda wrote: > I thought about this more overnight and I'm more convinced that lit and > lldb-mi make a great pair. lldb-mi is a programmatic text format that > isn't subject to the whims of command-line UI design over the years; well > tests written in terms of MI will be resilient and stable. lldb-mi MUCH > more closely matches a non-lldb developer's view of how a debugger works. > The syntax is different from what they use, but it isn't hard to figure > out. I haven't touched MI in four or five years and it only took me a > couple minutes to figure out how to do run to a breakpoint and print a > variable: > > -file-exec-and-symbols "/tmp/a.out" > > -break-insert a.c:10 > > =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x00010f70",func="main",file="a.c",fullname="/tmp/a.c",line="10",times="0",original-location="a.c:10"} > > -exec-run > > -thread-info > > -exec-next > > -var-create var1 * c > > ^done,name="var1",numchild="0",value="12",type="int",thread-id="1",has_more="0" > > > (I omitted the responses from all the command except -break-insert and > -var-create to make it easier to read, but go run lldb-mi and play with it > yourself or read the gdb MI documentation on the web. It's a really simple > format to work in.) > > > Several of the proposal have been special commands in the lldb command > line driver that have non-changing output styles or the like. Why > re-invent something that already exists? This makes a ton more sense on > every front. Yes, it means you can't copy and paste your lldb debug > session into a test case --- but we've all agreed that that's not something > we want anyone doing anyway. > > I'm in favor of lit + lldb-mi and think this would add a lot of value as > an additional way for people to write test cases. > > > J > > > > > On Sep 15, 2016, at 7:40 PM, Zachary Turner wrote: > > > > One thing I wonder about. It seems like everyone is mostly on the same > page about command line output . > > > > What about input? Would anyone have objections to a test which ran a few > commands to get the debugger into a particular state before doing something > to verify the output? Let's assume I'm waving my hands about this last step > but that it doesn't involve scraping the output of a debugger command > > > > Maybe this will never even be an issue, but I just want to make sure > everyone is on the same page and that the objections are: > > > > a) specific to command OUTPUT, not input (i.e. it's ok to have a test > run "break set -n main") > > b) specific to *non trivial* output (checking that "p 5" displays 5 is > ok) > > c) specific to the the output of the *user* command line interface, so > that some hypothetical other command line interface (which again I'm being > hand wavy about) would not be subject to the same objections. > > > > > > On Thu, Sep 15, 2016 at 7:28 PM Jason Molenda > wrote: > > > > > On Sep 15, 2016, at 8:02 AM, Zachary Turner > wrote: > > > > > > > > > It sounds like your goal is also "tests have to use the SB API and no > other API", which if so I think that's counterproductive. More > productive, IMO, would be being open to any alternative that addresses the > concerns you have with command-line tests. There are more than 2 ways to > skin a cat, so to speak. > > > > Thinking about this a bit, another approach would be to do lit tests on > top of lldb-mi. MI is a structured format for the debugger and a UI to > communicate back and forth with a simple text markup language (it would be > JSON if it were being done today, but it was added to gdb eighteen years > ago, so it's not). The commands correspond to the commands a debugger user > would think to use -- no need to understand the structure of how lldb is > implemented, like with the SB API. The format is a little unusual for a > human to type, but back when we supported gdb at Apple we worked in MI all > the time (it was used to communicate between Xcode, our IDE, and gdb) by > hand when testing and it was fine. "-exec-run" instead of run, that kin
[Lldb-commits] [PATCH] D24694: [LLDB] Fix Clang initialization and Clang-tidy modernize-use-nullptr warnings
Eugene.Zelenko created this revision. Eugene.Zelenko added reviewers: labath, zturner. Eugene.Zelenko added a subscriber: lldb-commits. Eugene.Zelenko set the repository for this revision to rL LLVM. Looks like RegisterContextNetBSD_x86_64.cpp should use RegisterInfos_x86_64.h as much as possible, but I think will be good idea if platform maintainer will take care about this. Repository: rL LLVM https://reviews.llvm.org/D24694 Files: source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp source/Plugins/Process/Utility/RegisterInfos_i386.h source/Plugins/Process/Utility/RegisterInfos_x86_64.h Index: source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp === --- source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp +++ source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp @@ -1,15 +1,16 @@ -//===-- RegisterContextNetBSD_x86_64.cpp --*- C++ -*-===// +//===-- RegisterContextNetBSD_x86_64.cpp *- C++ -*-===// // // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. // -//===-===// +//===--===// +#include #include -#include +#include "llvm/ADT/Triple.h" #include "llvm/Support/Compiler.h" #include "RegisterContextNetBSD_x86_64.h" @@ -89,10 +90,10 @@ LLVM_EXTENSION offsetof(XSAVE, ymmh[0]) + (32 * reg_index)) // Number of bytes needed to represent a FPR. -#define FPR_SIZE(reg) sizeof(((FXSAVE *)NULL)->reg) +#define FPR_SIZE(reg) sizeof(((FXSAVE *)nullptr)->reg) // Number of bytes needed to represent the i'th FP register. -#define FP_SIZE sizeof(((MMSReg *)NULL)->bytes) +#define FP_SIZE sizeof(((MMSReg *)nullptr)->bytes) // Number of bytes needed to represent an XMM register. #define XMM_SIZE sizeof(XMMReg) @@ -105,101 +106,106 @@ // Note that the size and offset will be updated by platform-specific classes. #define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4) \ {\ -#reg, alt, sizeof(((GPR *) NULL)->reg),\ +#reg, alt, sizeof(((GPR *)nullptr)->reg), \ GPR_OFFSET(reg), eEncodingUint, eFormatHex, \ {kind1, kind2, kind3, kind4, \ lldb_##reg##_x86_64 }, \ - NULL, NULL \ + nullptr, nullptr, nullptr, 0 \ } #define DEFINE_FPR(name, reg, kind1, kind2, kind3, kind4) \ {\ -#name, NULL, FPR_SIZE(reg), FPR_OFFSET(reg), eEncodingUint, eFormatHex,\ +#name, nullptr, FPR_SIZE(reg), FPR_OFFSET(reg), eEncodingUint, eFormatHex, \ {kind1, kind2, kind3, kind4,\ lldb_##name##_x86_64 },\ -NULL, NULL \ +nullptr, nullptr, nullptr, 0 \ } #define DEFINE_FP_ST(reg, i) \ {\ -#reg #i, NULL, FP_SIZE,\ +#reg #i, nullptr, FP_SIZE, \ LLVM_EXTENSION FPR_OFFSET( \ stmm[i]), eEncodingVector, eFormatVectorOfUInt8, \ {dwarf_st##i##_x86_64, dwarf_st##i##_x86_64, LLDB_INVALID_REGNUM, \ LLDB_INVALID_REGNUM, lldb_st##i##_x86_64 }, \ - NULL, NULL\ + nullptr, nullptr, nullptr, 0 \ } #define DEFINE_FP_MM(reg, i) \ {\ -#reg #i, NULL, sizeof(uint64_t), \ +#reg #i, nullptr, sizeof(uint64_t),\ LLVM_EXTENSION FPR_OFFSET( \ stmm[i]), eEncodingUint, eFormatHex, \ {dwarf_mm##i##_x86_64, dwarf_mm##i##_x86_64, \
[Lldb-commits] [lldb] r281799 - Convert many functions to use StringRefs.
Author: zturner Date: Fri Sep 16 21:00:02 2016 New Revision: 281799 URL: http://llvm.org/viewvc/llvm-project?rev=281799&view=rev Log: Convert many functions to use StringRefs. Where possible, remove the const char* version. To keep the risk and impact here minimal, I've only done the simplest functions. In the process, I found a few opportunities for adding some unit tests, so I added those as well. Tested on Windows, Linux, and OSX. Added: lldb/trunk/unittests/Breakpoint/ lldb/trunk/unittests/Breakpoint/BreakpointIDTest.cpp lldb/trunk/unittests/Breakpoint/CMakeLists.txt lldb/trunk/unittests/Platform/ lldb/trunk/unittests/Platform/CMakeLists.txt lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h lldb/trunk/include/lldb/Interpreter/Args.h lldb/trunk/include/lldb/Target/Language.h lldb/trunk/source/Breakpoint/Breakpoint.cpp lldb/trunk/source/Breakpoint/BreakpointID.cpp lldb/trunk/source/Breakpoint/BreakpointIDList.cpp lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp lldb/trunk/source/Interpreter/Args.cpp lldb/trunk/source/Interpreter/OptionGroupPlatform.cpp lldb/trunk/source/Interpreter/OptionValueLanguage.cpp lldb/trunk/source/Interpreter/Property.cpp lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/trunk/source/Target/Language.cpp lldb/trunk/unittests/CMakeLists.txt lldb/trunk/unittests/Interpreter/TestArgs.cpp Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointID.h?rev=281799&r1=281798&r2=281799&view=diff == --- lldb/trunk/include/lldb/Breakpoint/BreakpointID.h (original) +++ lldb/trunk/include/lldb/Breakpoint/BreakpointID.h Fri Sep 16 21:00:02 2016 @@ -17,6 +17,8 @@ #include "lldb/lldb-private.h" +#include "llvm/ADT/StringRef.h" + namespace lldb_private { //-- @@ -87,8 +89,7 @@ public: /// \b true if the name is a breakpoint name (as opposed to an ID or /// range) false otherwise. //-- - // TODO: Convert this function to use a StringRef. - static bool StringIsBreakpointName(const char *name, Error &error); + static bool StringIsBreakpointName(llvm::StringRef str, Error &error); //-- /// Takes a breakpoint ID and the breakpoint location id and returns Modified: lldb/trunk/include/lldb/Interpreter/Args.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=281799&r1=281798&r2=281799&view=diff == --- lldb/trunk/include/lldb/Interpreter/Args.h (original) +++ lldb/trunk/include/lldb/Interpreter/Args.h Fri Sep 16 21:00:02 2016 @@ -394,9 +394,8 @@ public: static uint32_t StringToGenericRegister(llvm::StringRef s); - // TODO: Update to take a StringRef - static const char *StringToVersion(const char *s, uint32_t &major, - uint32_t &minor, uint32_t &update); + static bool StringToVersion(llvm::StringRef string, uint32_t &major, + uint32_t &minor, uint32_t &update); static const char *GetShellSafeArgument(const FileSpec &shell, const char *unsafe_arg, Modified: lldb/trunk/include/lldb/Target/Language.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Language.h?rev=281799&r1=281798&r2=281799&view=diff == --- lldb/trunk/include/lldb/Target/Language.h (original) +++ lldb/trunk/include/lldb/Target/Language.h Fri Sep 16 21:00:02 2016 @@ -142,8 +142,8 @@ public: // These are accessors for general information about the Languages lldb knows // about: - // TODO: Convert this to using a StringRef. static lldb::LanguageType GetLanguageTypeFromString(const char *string); + static lldb::LanguageType GetLanguageTypeFromString(llvm::StringRef string); static const char *GetNameForLanguageType(lldb::LanguageType language); Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=281799&r1=281798&r2=281799&view
[Lldb-commits] [lldb] r281804 - Fix boolean logic error in BreakpointID.
Author: zturner Date: Fri Sep 16 21:34:40 2016 New Revision: 281804 URL: http://llvm.org/viewvc/llvm-project?rev=281804&view=rev Log: Fix boolean logic error in BreakpointID. Modified: lldb/trunk/source/Breakpoint/BreakpointID.cpp Modified: lldb/trunk/source/Breakpoint/BreakpointID.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointID.cpp?rev=281804&r1=281803&r2=281804&view=diff == --- lldb/trunk/source/Breakpoint/BreakpointID.cpp (original) +++ lldb/trunk/source/Breakpoint/BreakpointID.cpp Fri Sep 16 21:34:40 2016 @@ -113,7 +113,7 @@ bool BreakpointID::StringIsBreakpointNam return false; // First character must be a letter or _ - if (!isalpha(str[0]) || str[0] != '_') + if (!isalpha(str[0]) && str[0] != '_') return false; // Cannot contain ., -, or space. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits