Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Zachary Turner via lldb-commits
Also, it would be nice (but again this isn't immediately affecting me so I can't really put too much pressure here) if it continued to return None in case of error. Any code that would be affected by changing the error return value from None to "" was very likely broken already and this just

Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Jason Molenda via lldb-commits
Yeah, I'm going to check in one based on my little example program. The only tricky bit is > > const char *invalid_memory_string = (char*)0x100; // lower 4k is always > > PAGEZERO & unreadable on darwin I'll need some address that is unmapped for other platforms. Maybe address -1 or

Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Zachary Turner via lldb-commits
Probably obvious, can you add some tests to verify the new behavior? On Wed, Nov 15, 2017 at 7:47 PM Zachary Turner wrote: > Yea, if there is a valid string there, it should definitely be returning > "", hard to argue with that. > > On Wed, Nov 15, 2017 at 7:06 PM Jason

Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Zachary Turner via lldb-commits
Yea, if there is a valid string there, it should definitely be returning "", hard to argue with that. On Wed, Nov 15, 2017 at 7:06 PM Jason Molenda wrote: > The general point you're making is reasonable, and something like > Thread::GetStopDescription is not clear which the

Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Jason Molenda via lldb-commits
The general point you're making is reasonable, and something like Thread::GetStopDescription is not clear which the correct behavior should be. But I think we can all agree that Process::ReadCStringFromMemory() returning a None object when there is an empty c-string is incorrect. People are

Re: [Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Zachary Turner via lldb-commits
I don't really feel strongly about how you fix it. I'm sure there was a good reason for making it do that, but at this point I don't remember what it is and I don't think undoing it will cause a problem. That said, part of the difficulty of having an API such as this is that Hyrum's Law

[Lldb-commits] Need to change swig typemap for reading C strings

2017-11-15 Thread Jason Molenda via lldb-commits
Hi Zachary, reviving a problem I initially found a year ago -- in r253487 where a swig typemap was changed for string reading methods. The problem we're seeing with this change is that SBProcess::ReadCStringFromMemory() now returns a None object when you try to read a zero-length string, and

[Lldb-commits] [lldb] r318357 - Fix alignment of arm64 fpu register context structure

2017-11-15 Thread Jason Molenda via lldb-commits
Author: jmolenda Date: Wed Nov 15 16:50:29 2017 New Revision: 318357 URL: http://llvm.org/viewvc/llvm-project?rev=318357=rev Log: Fix alignment of arm64 fpu register context structure so it has the same padding as the kernel's definition which is written in terms of uint128_t. Original patch by

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. On second thought I actually think the strategy used here is fine. But we have `llvm::WritableBinaryStream` which could server as a single base interface for a `mapped_file_region` based implementation as well as a malloc-based implementation. For the malloc based

Re: [Lldb-commits] [lldb] r318262 - Roll back r318260 because it is causing the windows bot to

2017-11-15 Thread Jason Molenda via lldb-commits
> On Nov 15, 2017, at 1:38 AM, Pavel Labath wrote: > > On 15 November 2017 at 03:41, Jason Molenda via lldb-commits > wrote: >> Author: jmolenda >> Date: Tue Nov 14 19:41:47 2017 >> New Revision: 318262 >> >> URL:

[Lldb-commits] [lldb] r318348 - [POSIX] Replace assert with llvm_unreachable(). NFCI.

2017-11-15 Thread Davide Italiano via lldb-commits
Author: davide Date: Wed Nov 15 15:39:41 2017 New Revision: 318348 URL: http://llvm.org/viewvc/llvm-project?rev=318348=rev Log: [POSIX] Replace assert with llvm_unreachable(). NFCI. Modified: lldb/trunk/source/Host/posix/HostThreadPosix.cpp Modified:

Re: [Lldb-commits] [lldb] r318262 - Roll back r318260 because it is causing the windows bot to

2017-11-15 Thread Zachary Turner via lldb-commits
It sounds to me like what you *really* need is for the VReg type itself to be aligned 16. How about changing this: struct VReg { uint8_t bytes[16]; }; to this: struct VReg { llvm::AlignedCharArray<16, 16> bytes; }; Then the FPU struct can remain unchanged. MSVC does have a 128 bit

Re: [Lldb-commits] [lldb] r318262 - Roll back r318260 because it is causing the windows bot to

2017-11-15 Thread Jason Molenda via lldb-commits
Hi Zachary, do you have a suggestion on how to specify this alignment in a way that works with MSVC? > On Nov 14, 2017, at 7:41 PM, Jason Molenda via lldb-commits > wrote: > > Author: jmolenda > Date: Tue Nov 14 19:41:47 2017 > New Revision: 318262 > > URL:

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D40079#926328, @clayborg wrote: > I had suggested in https://reviews.llvm.org/D38142 that we have ObjectFileELF > check the file type of the file and only map with write abilities if the ELF > file is an object file since that is the only

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via lldb-commits
On 15 November 2017 at 18:11, Zachary Turner wrote: > > > On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath wrote: >> >> On 15 November 2017 at 17:42, Zachary Turner wrote: >> > Can we just extend llvm's mapped_file_region to support a

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Greg Clayton via lldb-commits
> On Nov 15, 2017, at 10:11 AM, Zachary Turner wrote: > > > > On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath > wrote: > On 15 November 2017 at 17:42, Zachary Turner > wrote: > >

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via lldb-commits
On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath wrote: > On 15 November 2017 at 17:42, Zachary Turner wrote: > > Can we just extend llvm's mapped_file_region to support a boolean > Writable > > flag? > > > mapped_file_region already can be writable. The

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I had suggested in https://reviews.llvm.org/D38142 that we have ObjectFileELF check the file type of the file and only map with write abilities if the ELF file is an object file since that is the only time we need relocations. If we can pass a flag through to indicate

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via lldb-commits
On 15 November 2017 at 17:42, Zachary Turner wrote: > Can we just extend llvm's mapped_file_region to support a boolean Writable > flag? > mapped_file_region already can be writable. The feature it is missing is the ability to *not* use mmap. And that's not a good idea, as the

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via lldb-commits
Can we just extend llvm's mapped_file_region to support a boolean Writable flag? On Wed, Nov 15, 2017 at 8:03 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath created this revision. > Herald added subscribers: krytarowski, arichardson, aprantl, emaste. > > We sometimes

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. Herald added subscribers: krytarowski, arichardson, aprantl, emaste. We sometimes need to write to the object file we've mapped into memory, generally to apply relocations to debug info sections. We've had that ability before, but with the introduction of

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

2017-11-15 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha added inline comments. Comment at: source/Symbol/ObjectFile.cpp:695 + "One or more breakpoints intersect section '%s'", + section_sp->GetName().AsCString()); return error; abidh wrote: > If WriteMemory is not

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

2017-11-15 Thread Hafiz Abid Qadeer via Phabricator via lldb-commits
abidh added inline comments. Comment at: source/Symbol/ObjectFile.cpp:695 + "One or more breakpoints intersect section '%s'", + section_sp->GetName().AsCString()); return error; If WriteMemory is not setting the error in some

[Lldb-commits] [PATCH] D39681: Implement core dump debugging for PPC64le

2017-11-15 Thread Kamil Rytarowski via Phabricator via lldb-commits
krytarowski added inline comments. Comment at: source/Plugins/Process/elf-core/elf-core-enums.h:14 +/// Core files PT_NOTE segment descriptor types +enum { + NT_PRSTATUS = 1, labath wrote: > krytarowski wrote: > > alexandreyy wrote: > > > krytarowski wrote: > >

[Lldb-commits] [PATCH] D39681: Implement core dump debugging for PPC64le

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Also, I have trouble downloading the core file from phabricator. Alexandre, can you sent them to me directly? https://reviews.llvm.org/D39681 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D39681: Implement core dump debugging for PPC64le

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: source/Plugins/Process/elf-core/elf-core-enums.h:14 +/// Core files PT_NOTE segment descriptor types +enum { + NT_PRSTATUS = 1, krytarowski wrote: > alexandreyy wrote: > > krytarowski wrote: > > > alexandreyy wrote: > >

Re: [Lldb-commits] [lldb] r318262 - Roll back r318260 because it is causing the windows bot to

2017-11-15 Thread Pavel Labath via lldb-commits
On 15 November 2017 at 03:41, Jason Molenda via lldb-commits wrote: > Author: jmolenda > Date: Tue Nov 14 19:41:47 2017 > New Revision: 318262 > > URL: http://llvm.org/viewvc/llvm-project?rev=318262=rev > Log: > Roll back r318260 because it is causing the windows bot

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

2017-11-15 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha added a comment. Still have a feeling that it is not enough to clarify for function users that "if (bytes_written != bytes_to_write)" is not always criteria of error, as well as "if (error.Success())" doesn't mean that whole area was written to memory... Repository: rL

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

2017-11-15 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha updated this revision to Diff 122972. tatyana-krasnukha added a comment. Same for comment in the header file. Repository: rL LLVM https://reviews.llvm.org/D39967 Files: include/lldb/Target/Process.h source/Target/Process.cpp Index: source/Target/Process.cpp