Author: friss
Date: Thu Feb 22 21:29:27 2018
New Revision: 325862
URL: http://llvm.org/viewvc/llvm-project?rev=325862&view=rev
Log:
Fix TestMultithreaded when there's no debugserver specified
r325858 was bogus and would error out with a KeyError when --server
was not passed to dotest.py.
Modifie
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325859: Fix TestUbsanBasic (authored by friss, committed by
).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43577?vs=135263&id=135590#toc
Repository:
rL
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325858: Fix TestMultithreaded when specifying an alternative
debugserver. (authored by friss, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D4354
Author: friss
Date: Thu Feb 22 21:03:09 2018
New Revision: 325858
URL: http://llvm.org/viewvc/llvm-project?rev=325858&view=rev
Log:
Fix TestMultithreaded when specifying an alternative debugserver.
Summary:
This test launches a helper that uses the debugserver. The environment
variable sepcifying
Author: friss
Date: Thu Feb 22 21:03:10 2018
New Revision: 325859
URL: http://llvm.org/viewvc/llvm-project?rev=325859&view=rev
Log:
Fix TestUbsanBasic
Summary:
Potentially due to the recent testuite refactorings, this test now reports
a full absolute path but expect just the filename. For some re
labath added a comment.
While we wait for the verdict on the template stuff, it occurred to me that you
don't need the PerformCleanup bool member as std::function has an "empty"
state. So the destructor can just do `if(Cleanup) Cleanup()` and the disable
function can reset the cleanup object. (
Author: emaste
Date: Thu Feb 22 18:50:07 2018
New Revision: 325856
URL: http://llvm.org/viewvc/llvm-project?rev=325856&view=rev
Log:
remove FreeBSD xfail from lit TestCallStdStringFunction
This test is consistently reporting unexpected pass for me, and the
expectedFailure decorator was removed fr
vsk added a comment.
In https://reviews.llvm.org/D43662#1016950, @labath wrote:
> In https://reviews.llvm.org/D43662#1016939, @vsk wrote:
>
> > > What do you think about a syntax like:
> > >
> > > lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp
> > > constructor creates t
I wouldn’t use std::bind though, just make a lambda.
On Thu, Feb 22, 2018 at 6:06 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> In https://reviews.llvm.org/D43662#1016939, @vsk wrote:
>
> > > What do you think about a syntax like:
> > >
> > > lldb
labath added a comment.
In https://reviews.llvm.org/D43662#1016939, @vsk wrote:
> > What do you think about a syntax like:
> >
> > lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp
> > constructor creates the std::function internally
>
> @labath I find the current version o
vsk added a comment.
> Couldn't we just add a creator helper so that you can say:
>
> auto cleanup = makeCleanupRAII([&] {
>
> process_sp->Destroy(/*force_kill=*/false);
> debugger.StopEventHandlerThread();
> });
@zturner In this version of the patch, CleanUp accepts a std::function,
labath added a comment.
Btw, I think we should also move the CleanUp class out of the lldb_utility and
into lldb_private namespace. we have been slowly getting rid of these...
https://reviews.llvm.org/D43662
___
lldb-commits mailing list
lldb-commi
labath added a comment.
What do you think about a syntax like:
lldb_utility::CleanUp cleanup_our(::close, our_socket); // The CleanUp
constructor creates the std::function internally
This would be a generalization of your syntax, so you could still use a lambda
when needed, but you could avo
On Wed, Feb 21, 2018 at 10:52 AM, Davide Italiano wrote:
>
>
> On Tue, Feb 20, 2018 at 10:20 PM, Frederic Riss via lldb-commits
> wrote:
>>
>> Author: friss
>> Date: Tue Feb 20 22:20:03 2018
>> New Revision: 325666
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=325666&view=rev
>> Log:
>> Fix
Author: davide
Date: Thu Feb 22 17:33:20 2018
New Revision: 325851
URL: http://llvm.org/viewvc/llvm-project?rev=325851&view=rev
Log:
[testsuite] Throw away test/debug_info/apple_types.
This test was only testing that clang produced the correct informations
for __apple accelerated tables. So, it's
zturner added a comment.
Couldn't we just add a creator helper so that you can say:
auto cleanup = makeCleanupRAII([&] {
process_sp->Destroy(/*force_kill=*/false);
debugger.StopEventHandlerThread();
});
https://reviews.llvm.org/D43662
___
vsk created this revision.
vsk added reviewers: zturner, labath, davide, JDevlieghere.
Removing the template arguments and most of the mutating methods from
CleanUp makes it easier to understand and reuse.
In its present state, CleanUp would be too cumbersome to adapt to cases
where multiple obje
labath added a comment.
Yes, the property would probably be a member variable of some object (maybe
even of the parent property, although we would also want to have a setup where
the properties can be declared outside of the parent object (for plugins,
etc.)). Then something would need to insta
aprantl added a comment.
Nice!
https://reviews.llvm.org/D43596
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
I like Pavel's template syntax. It's not clear to me how to best implement the
registering of the properties this way though. I think we want to avoid static
intializers. Given how TargetProperties::TargetProperties() is implemented, I
could imagine we could just make t
labath updated this revision to Diff 135568.
labath added a comment.
This adds a test which looks up a type with unicode characters. I have verified
this test does not pass when run against a pre-D42740 compiler.
https://reviews.llvm.org/D43596
Files:
include/lldb/Core/MappedHash.h
packages
jingham added a comment.
The question I'm asking is if I didn't know the name of the function, but I had
found the found the string name of the property in the defs file, how would I
figure out the name of the setter and getter? But I see that you use Get##ID
so it's always going to be {Get,Se
labath added a comment.
I'm not against this in any way, but my long-term goal for this would be so
that I can declare a property with something like:
Property InputPath(ParentProperty, "input-path",
"description-string")
where the Property class would know have an appropriate conversion ope
Author: vedantk
Date: Thu Feb 22 16:29:40 2018
New Revision: 325847
URL: http://llvm.org/viewvc/llvm-project?rev=325847&view=rev
Log:
Delete some unused #includes of CleanUp.h, NFC
Modified:
lldb/trunk/source/Commands/CommandCompletions.cpp
lldb/trunk/source/Host/common/Host.cpp
lldb/
labath added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161
+ class Guard : private std::unique_lock {
+using std::unique_lock::unique_lock;
+ };
zturner wrote:
> Err, I meant to just deleted the `Guard` class entirely and
aprantl added a comment.
In https://reviews.llvm.org/D43647#1016722, @jingham wrote:
> What would I have to figure out if I wanted to stop where one of these
> properties was actually set or read? That's something I've had to do many's
> the time... This task is straightforward in the origina
jingham added a comment.
What would I have to figure out if I wanted to stop where one of these
properties was actually set or read? That's something I've had to do many's
the time... This task is straightforward in the original, and if it's not too
bad to cons up the name from this macro goo
tatyana-krasnukha updated this revision to Diff 135537.
tatyana-krasnukha added a comment.
Fix naming
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/BreakpointSiteList.h
source/Breakpoint/BreakpointSiteList.cpp
source/Target/Process.cpp
unittests/Process/CMakeLists.txt
Author: vedantk
Date: Thu Feb 22 15:48:21 2018
New Revision: 325841
URL: http://llvm.org/viewvc/llvm-project?rev=325841&view=rev
Log:
[ObjC] Fix the NSConcreteData formatter and test it
The length field of an NSConcreteData lives one word past the start of
the object, not two.
Modified:
lld
aprantl created this revision.
aprantl added reviewers: jingham, jasonmolenda, clayborg.
Herald added a subscriber: llvm-commits.
I recently had to add a new property and was annoyed by how much repetitive
code I needed to write manually.
This patch applies a common pattern used all over LLVM to
tatyana-krasnukha updated this revision to Diff 135533.
tatyana-krasnukha added a comment.
Remove Guard class completely
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/BreakpointSiteList.h
source/Breakpoint/BreakpointSiteList.cpp
source/Target/Process.cpp
unittests/Proce
tatyana-krasnukha added inline comments.
Comment at: source/Target/Process.cpp:2437
+ for (auto &bp_site : enabled_bp_sites_in_range)
+update_status(DisableSoftwareBreakpoint(bp_site.get()));
xiaobai wrote:
> If disabling any breakpoint fails, the status's
zturner added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161
+ class Guard : private std::unique_lock {
+using std::unique_lock::unique_lock;
+ };
Err, I meant to just deleted the `Guard` class entirely and return
`llvm::
tatyana-krasnukha added a comment.
In https://reviews.llvm.org/D39967#1016533, @zturner wrote:
> It might be true that allowing the use of manual `lock` and `unlock` is
> unsafe, but adding additional code also has some cost.
Just look: only 3 additional lines of code;)
https://reviews.llvm.
tatyana-krasnukha updated this revision to Diff 135527.
tatyana-krasnukha added a comment.
Update according suggestions above
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/BreakpointSiteList.h
source/Breakpoint/BreakpointSiteList.cpp
source/Target/Process.cpp
unittests/
xiaobai added inline comments.
Comment at: source/Breakpoint/BreakpointSiteList.cpp:191
if (lower != m_bp_site_list.begin()) {
-collection::const_iterator prev_pos = lower;
-prev_pos--;
+auto prev_pos = std::prev(lower);
const BreakpointSiteSP &prev_bp = (*pre
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325835: Fix TestSBData.py on Windows (authored by amccarth,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43532?vs=135148&id=135523#toc
Rep
Author: amccarth
Date: Thu Feb 22 14:47:47 2018
New Revision: 325836
URL: http://llvm.org/viewvc/llvm-project?rev=325836&view=rev
Log:
Fix TestMoveNearest on Windows
The header file for the DLL tried to declare inline functions and a local
function as dllexport which broke the compile and link.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325836: Fix TestMoveNearest on Windows (authored by
amccarth, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43600?vs=135345&id=135524#toc
R
tatyana-krasnukha added inline comments.
Comment at: source/Breakpoint/BreakpointSiteList.cpp:191
if (lower != m_bp_site_list.begin()) {
-collection::const_iterator prev_pos = lower;
-prev_pos--;
+auto prev_pos = std::prev(lower);
const BreakpointSiteSP &prev_
Author: amccarth
Date: Thu Feb 22 14:47:14 2018
New Revision: 325835
URL: http://llvm.org/viewvc/llvm-project?rev=325835&view=rev
Log:
Fix TestSBData.py on Windows
Ensure that the test data is an array of bytes rather than a string that gets
encoded differently between Python 2 and Python 3.
Dif
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Ok, that's the piece that was missing since the code for `foo.cpp` and
`main.cpp` weren't part of the patch.
https://reviews.llvm.org/D43600
_
zturner added a comment.
Personally I think it would be clearer to just use `std::unique_lock<>`.
Already it's locking the mutex twice, once with a `lock_guard` and once when
creating a `BreakpointSiteList::Guard`. Which works I guess because it's a
recursive mutex, but it's still confusing.
amccarth added a comment.
call_foo1() is defined in foo.cpp, which is built into a DLL, and thus needs
the __declspec(dllimport/dllexport) from the LLDB_TEST_API. foo.h defines the
API for the DLL.
call_foo2() is defined in main.cpp, which is built into an executable that
depends on foo.dll.
xiaobai added inline comments.
Comment at: source/Breakpoint/BreakpointSiteList.cpp:191
if (lower != m_bp_site_list.begin()) {
-collection::const_iterator prev_pos = lower;
-prev_pos--;
+auto prev_pos = std::prev(lower);
const BreakpointSiteSP &prev_bp = (*pre
tatyana-krasnukha added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175
+ class Guard final {
+std::recursive_mutex *m_mutex;
- typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp,
-
zturner added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175
+ class Guard final {
+std::recursive_mutex *m_mutex;
- typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp,
- void *bat
tatyana-krasnukha added a comment.
In https://reviews.llvm.org/D39967#1015860, @clayborg wrote:
> So one issue with this is some things, like GDB servers, might not need you
> to disable breakpoints in a range because they actually set the breakpoint
> for you and you don't know what opcode the
tatyana-krasnukha updated this revision to Diff 135518.
tatyana-krasnukha added a comment.
Use Disable|EnableBreakpointSite instead of Disable|EnableSoftwareBreakpoint.
Make BreakpointSiteList::ForEach callbacks return bool.
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/Break
zturner added a comment.
How is `call_foo2` any different than `call_foo1`, which was not removed from
this patch?
https://reviews.llvm.org/D43600
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
amccarth added a comment.
In https://reviews.llvm.org/D43600#1016483, @zturner wrote:
> Do we not need `call_foo2` anymore?
call_foo2 is defined in the executable, so attempting to report it as dllexport
from the DLL is nonsensical and causes link-time errors.
https://reviews.llvm.org/D43600
zturner added a comment.
Do we not need `call_foo2` anymore?
https://reviews.llvm.org/D43600
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jingham added a comment.
The point of --batch is that if the program crashes, control is transferred
from the script to the user so that they can inspect the crash. lit would just
hang at that point, right?
I worry that people will use this RUN example as an template, and then make a
test tha
zturner added a comment.
Actually I may have misunderstood the help text. It sounds like it may be
referring to a crash of the inferior, not a crash of LLDB itself. If the test
contains no run commands (as this one doesn't), then it doesn't seem like
there's any risk of this happening, and th
zturner added a comment.
If your commands are A, B, C, and D but A crashes and returns to the
interactive prompt, will it still try to execute B, C, and D? If so then I
guess that would work (I haven't tried). OTOH, there's a risk of people
forgetting to do this (but I'm not sure if the risk
jingham added a comment.
Rather than adding another flag, can't you just put -o quit at the end of your
RUN line?
Repository:
rL LLVM
https://reviews.llvm.org/D43471
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org
owenpshaw added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3478
+auto header = GetProgramHeaderByIndex(i);
+if (header->p_paddr != 0)
+ return true;
clayborg wrote:
> Can't p_paddr be zero and still be valid? Would a
labath added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175
+ class Guard final {
+std::recursive_mutex *m_mutex;
- typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp,
- void *bato
labath added inline comments.
Comment at: source/Core/Module.cpp:1283-1284
if (!m_sections_ap) {
-ObjectFile *obj_file = GetObjectFile();
-if (obj_file != nullptr)
+if (ObjectFile *obj_file = GetObjectFile())
obj_file->CreateSections(*GetUnifiedSectionList()
friss added a comment.
Note that this code path is only triggered when importing debug info from a
different AST context, it is not the common codepath. The issue in this case is
that LLDB is crashing when using the incomplete Decl as the DeclContext for
another one. I guess I could add calls t
zturner added a comment.
One issue I can see with this is that if you check the documentation of -b it
says this:
-b
--batch
Tells the debugger to run the commands from -s, -S, -o & -O, and
then quit. However if any run command stopped due to a signal or
crash, the debu
clayborg added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3478
+auto header = GetProgramHeaderByIndex(i);
+if (header->p_paddr != 0)
+ return true;
Can't p_paddr be zero and still be valid? Would a better test be
"h
clayborg added a comment.
Pavel: I would vote for the same thing, if the test runs fine, nuke the
directory, if it fails for any reason, leave it around so we can do post
mortem. Might be nice to copy the logs over into the build directory if things
fail so it is really easy to just go into the
clayborg added a comment.
Back from vacation, sorry for the delay. Only one question on who is actually
responsible for creating the sections. I vote to let
SymbolVendor::CreateSection() always do the work. Let me know what you think.
Comment at: source/Core/Module.cpp:1283-1
aprantl added a comment.
In https://reviews.llvm.org/D43596#1015903, @clayborg wrote:
> The only input we currently use this on is DWARF and I believe all DWARF
> strings are UTF8 so this should all work correctly, no?
The hashing algorithms produced different hashes for characters with bit 7
clayborg added a comment.
The only input we currently use this on is DWARF and I believe all DWARF
strings are UTF8 so this should all work correctly, no?
https://reviews.llvm.org/D43596
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
htt
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Sean was the person with the most experience working on the expression parser
and he wrote the clang AST importer, but Jim is now the expression parser
expert. Jim: and ideas on
JDevlieghere added a comment.
In https://reviews.llvm.org/D43596#1015848, @clayborg wrote:
> No need to add additional dSYM tests, because all dSYM tests will fail if the
> hashing isn't correct.
Maybe I misunderstand, but https://reviews.llvm.org/D42740 suggested that this
never worked (or m
clayborg added a comment.
We could add a plugin setting that could be set with "setting set" that would
take precedence over the environment variable is we never need to. Then we
could actually run using different GDB server binaries in the same test run if
needed.
https://reviews.llvm.org/D4
clayborg added a comment.
So one issue with this is some things, like GDB servers, might not need you to
disable breakpoints in a range because they actually set the breakpoint for you
and you don't know what opcode they used. I am hoping this works with that and
doesn't cause us to manually wr
clayborg accepted this revision.
clayborg added a comment.
No need to add additional dSYM tests, because all dSYM tests will fail if the
hashing isn't correct.
https://reviews.llvm.org/D43596
___
lldb-commits mailing list
lldb-commits@lists.llvm.or
JDevlieghere added a reviewer: JDevlieghere.
JDevlieghere added a comment.
We probably want to have a test to ensure this stays in sync with the hashes we
generate for object files and dSYMs?
https://reviews.llvm.org/D43596
___
lldb-commits mailing
tatyana-krasnukha updated this revision to Diff 135397.
tatyana-krasnukha added a comment.
diff with more context
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/BreakpointSiteList.h
source/Breakpoint/BreakpointSiteList.cpp
source/Target/Process.cpp
unittests/Process/CMak
tatyana-krasnukha updated this revision to Diff 135387.
tatyana-krasnukha added a comment.
pass callbacks by const-reference
https://reviews.llvm.org/D39967
Files:
include/lldb/Breakpoint/BreakpointSiteList.h
source/Breakpoint/BreakpointSiteList.cpp
source/Target/Process.cpp
unittests/P
tatyana-krasnukha updated this revision to Diff 135370.
tatyana-krasnukha added a comment.
Herald added a subscriber: mgorny.
Disable breakpoints before writing memory and re-enable after.
I found that bp_site_list filled by BreakpointSiteList::FindInRange has its own
mutex and doesn’t hold mute
75 matches
Mail list logo
