[lldb-dev] [Bug 37485] LLDB reads wrong registers on 64bit Windows

2018-05-17 Thread via lldb-dev
https://bugs.llvm.org/show_bug.cgi?id=37485

Stella Stamenova  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 CC||sti...@microsoft.com
 Status|NEW |RESOLVED

--- Comment #3 from Stella Stamenova  ---
I committed your patch to the mainline. Thanks!

This fixed 8 or so tests in the lldbsuite as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Advice on architectures with multiple address spaces

2018-05-17 Thread Zdenek Prikryl via lldb-dev

Greg, Jim, what's your opinion here?

What about having the class Address (pretty much as it is right now) and 
the


struct AddressBase {
  lldb::addr_t m_address;
  lldb::as_t m_address_space;
  ...
}

Another question is, which classes/code should use Address, AddressBase, 
and addr_t. Do you have any idea here?


Zdenek

On 05/17/2018 01:38 PM, Pavel Labath wrote:

The Address class may be suitable for the higher layers of lldb, but I
don't think the it can ever be a blanket replacement for lldb::addr_t. It
has way too much smartness built-in. We use addr_t in a lot of places that
don't/shouldn't care about Targets, ExecutionContexts or Sections. All of
lldb-server is one of those places, but this is also true for any low-level
operation which only wants to work with real (virtual) addresses in the
process address space.

On the other hand, replacing addr_t with a lighweight struct which is just
adds some sort of an address space identifier seems like a useful thing,
and would go a long way towards bringing Harward architecture support to
lldb-server. (Note that we would still need an addr_t or something of that
form to name the type of the "address" member of the struct, but pretty
much all of the apis that currently take addr_t, could that the new struct
instead).


On Thu, 17 May 2018 at 12:01, Zdenek Prikryl via lldb-dev <
lldb-dev@lists.llvm.org> wrote:



On 04/19/2018 08:22 PM, Jim Ingham wrote:

On Apr 19, 2018, at 10:54 AM, Greg Clayton  wrote:




On Apr 19, 2018, at 10:35 AM, Jim Ingham  wrote:




On Apr 19, 2018, at 9:44 AM, Greg Clayton via lldb-dev <

lldb-dev@lists.llvm.org> wrote:




On Apr 19, 2018, at 6:51 AM, Zdenek Prikryl via lldb-dev <

lldb-dev@lists.llvm.org> wrote:

Hi lldb developers,

I've been researching using lldb + gdbserver stub that is based on

Harvard architecture with multiple address spaces (one program, multiple
data). The commonly adopted approach is that everything is mapped to a
single "virtual" address space. The stub reads/writes from/to the right
memory based on the "virtual" addresses. But I'd like to use real addresses
with address space id instead. So, I've started looking at what has to be
changed.

I've enhanced read/write commands (e.g. memory read --as  ...)

and RSP protocol (new packet) so that the stub can read/write properly.
That wasn't that complicated.

It might be nice to add a new RSP protocol packet that asks for the

address space names/values:

qGetAddressSpaces

which would return something like:

1:text;2:data1,3:data2

or it would return not supported. If we get a valid return value

from qGetAddressSpaces, then it enables the use of the new packet you added
above. Else it defaults to using the old memory read functions.



Now I've hit an issue with expressions (LLVMUserExpression.cpp) and

local variables (DWARFExpressions.cpp). There is a lot of memory read/write
functions that take just an address argument. Is the only way to go to
patch all these calls? Has anybody solved it differently?

My quick take is that any APIs that take just a lldb::addr_t would

need to take something like:

struct SpaceAddress {
static constexpr uint32_t kNoSpace = 0;
lldb::addr_t addr;
uint32_t space;
};


I'm curious why you are suggesting another kind of address, rather

than adding this functionality to Address?  When you actually go to resolve
an Address in a target with a process you should have everything you need
to know to give it the proper space.  Then fixing the expression evaluator
(and anything else that needs fixing) would be a matter of consistently
using Address rather than lldb::addr_t.  That seems general goodness, since
converting to an lldb::addr_t loses information.

If we accept lldb_private::Address in all APIs that take a

lldb::addr_t currently, then we need to always be able to get to the target
in case we need to add code to resolve the address everywhere. I am
thinking of SpaceAddress as an augmented lldb::addr_t instead of a section
+ offset style address. Also, there will be addresses in the code and data
that do not exist in actual sections. Not saying that you couldn't use
lldb_private::Address. I am open to suggestions though. So your though it
remove all API that take lldb::addr_t and use lldb_private::Address
everywhere all the time?

It has always bugged me that we have these two ways of specifying

addresses.  Are there many/any places that have to resolve an Address to a
real address in a process that don't have a Target readily available?  That
would surprise me.  I would much rather centralize on one way than adding a
third.

Jim

So, does it make sense to start with lldb::addr_t replacement? In other
words, replace all instances of lldb::addr_t with Address. It'd be the
first step and first patch towards to the ability to extend it in the
future, right?
Zdenek





Jim



We would need a default value for "space" (feel free to rename) that

indicates the default address space as most of our architectures would 

[lldb-dev] [Bug 37496] New: Sometimes LLDB freeze after launching process

2018-05-17 Thread via lldb-dev
https://bugs.llvm.org/show_bug.cgi?id=37496

Bug ID: 37496
   Summary: Sometimes LLDB freeze after launching process
   Product: lldb
   Version: 6.0
  Hardware: PC
OS: All
Status: NEW
  Severity: normal
  Priority: P
 Component: All Bugs
  Assignee: lldb-dev@lists.llvm.org
  Reporter: kenji.koyan...@gmail.com
CC: llvm-b...@lists.llvm.org

Created attachment 20313
  --> https://bugs.llvm.org/attachment.cgi?id=20313&action=edit
patch for 6.0.0

Sometimes(1 of 5-7 times) LLDB freeze at Process::WaitForProcessToStop after
launching process.
I am using lldb for Windows, but I think that it is not a OS-specific issue.

I inserted a log and tried to run LLDB.
Process::SetPublicState called with argument eStateLaunching -> eStateStopped
-> eStateLaunching.
I think there is a problem in the processing order of broadcast events.

It worked fine if consume the event before calling SetPublicState in
Process::Launch.
But this is not the best solution.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] [Bug 37495] New: LLDB shows wrong results when execute 'register read' at non-zero frame on Windows

2018-05-17 Thread via lldb-dev
https://bugs.llvm.org/show_bug.cgi?id=37495

Bug ID: 37495
   Summary: LLDB shows wrong results when execute 'register read'
at non-zero frame on Windows
   Product: lldb
   Version: 6.0
  Hardware: PC
OS: Windows NT
Status: NEW
  Severity: enhancement
  Priority: P
 Component: All Bugs
  Assignee: lldb-dev@lists.llvm.org
  Reporter: kenji.koyan...@gmail.com
CC: llvm-b...@lists.llvm.org

Created attachment 20312
  --> https://bugs.llvm.org/attachment.cgi?id=20312&action=edit
patch for 6.0.0

The result of 'register read' command at deeper than frame #0 was same as frame
#0.
Further, 'thread return' command had no effect on current thread.

I think that the behavior is different between TargetThreadWindows and
ThreadElfCore.
These commands worked correctly when I revised TargetThreadWindows with
reference to ThreadElfCore.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Advice on architectures with multiple address spaces

2018-05-17 Thread Pavel Labath via lldb-dev
The Address class may be suitable for the higher layers of lldb, but I
don't think the it can ever be a blanket replacement for lldb::addr_t. It
has way too much smartness built-in. We use addr_t in a lot of places that
don't/shouldn't care about Targets, ExecutionContexts or Sections. All of
lldb-server is one of those places, but this is also true for any low-level
operation which only wants to work with real (virtual) addresses in the
process address space.

On the other hand, replacing addr_t with a lighweight struct which is just
adds some sort of an address space identifier seems like a useful thing,
and would go a long way towards bringing Harward architecture support to
lldb-server. (Note that we would still need an addr_t or something of that
form to name the type of the "address" member of the struct, but pretty
much all of the apis that currently take addr_t, could that the new struct
instead).


On Thu, 17 May 2018 at 12:01, Zdenek Prikryl via lldb-dev <
lldb-dev@lists.llvm.org> wrote:


> On 04/19/2018 08:22 PM, Jim Ingham wrote:
> >
> >> On Apr 19, 2018, at 10:54 AM, Greg Clayton  wrote:
> >>
> >>
> >>
> >>> On Apr 19, 2018, at 10:35 AM, Jim Ingham  wrote:
> >>>
> >>>
> >>>
>  On Apr 19, 2018, at 9:44 AM, Greg Clayton via lldb-dev <
lldb-dev@lists.llvm.org> wrote:
> 
> 
> 
> > On Apr 19, 2018, at 6:51 AM, Zdenek Prikryl via lldb-dev <
lldb-dev@lists.llvm.org> wrote:
> >
> > Hi lldb developers,
> >
> > I've been researching using lldb + gdbserver stub that is based on
Harvard architecture with multiple address spaces (one program, multiple
data). The commonly adopted approach is that everything is mapped to a
single "virtual" address space. The stub reads/writes from/to the right
memory based on the "virtual" addresses. But I'd like to use real addresses
with address space id instead. So, I've started looking at what has to be
changed.
> >
> > I've enhanced read/write commands (e.g. memory read --as  ...)
and RSP protocol (new packet) so that the stub can read/write properly.
That wasn't that complicated.
>  It might be nice to add a new RSP protocol packet that asks for the
address space names/values:
> 
>  qGetAddressSpaces
> 
>  which would return something like:
> 
>  1:text;2:data1,3:data2
> 
>  or it would return not supported. If we get a valid return value
from qGetAddressSpaces, then it enables the use of the new packet you added
above. Else it defaults to using the old memory read functions.
> 
> 
> > Now I've hit an issue with expressions (LLVMUserExpression.cpp) and
local variables (DWARFExpressions.cpp). There is a lot of memory read/write
functions that take just an address argument. Is the only way to go to
patch all these calls? Has anybody solved it differently?
>  My quick take is that any APIs that take just a lldb::addr_t would
need to take something like:
> 
>  struct SpaceAddress {
>  static constexpr uint32_t kNoSpace = 0;
>  lldb::addr_t addr;
>  uint32_t space;
>  };
> 
> >>> I'm curious why you are suggesting another kind of address, rather
than adding this functionality to Address?  When you actually go to resolve
an Address in a target with a process you should have everything you need
to know to give it the proper space.  Then fixing the expression evaluator
(and anything else that needs fixing) would be a matter of consistently
using Address rather than lldb::addr_t.  That seems general goodness, since
converting to an lldb::addr_t loses information.
> >> If we accept lldb_private::Address in all APIs that take a
lldb::addr_t currently, then we need to always be able to get to the target
in case we need to add code to resolve the address everywhere. I am
thinking of SpaceAddress as an augmented lldb::addr_t instead of a section
+ offset style address. Also, there will be addresses in the code and data
that do not exist in actual sections. Not saying that you couldn't use
lldb_private::Address. I am open to suggestions though. So your though it
remove all API that take lldb::addr_t and use lldb_private::Address
everywhere all the time?
> > It has always bugged me that we have these two ways of specifying
addresses.  Are there many/any places that have to resolve an Address to a
real address in a process that don't have a Target readily available?  That
would surprise me.  I would much rather centralize on one way than adding a
third.
> >
> > Jim

> So, does it make sense to start with lldb::addr_t replacement? In other
> words, replace all instances of lldb::addr_t with Address. It'd be the
> first step and first patch towards to the ability to extend it in the
> future, right?

> Zdenek


> >
> >
> >>> Jim
> >>>
> >>>
>  We would need a default value for "space" (feel free to rename) that
indicates the default address space as most of our architectures would not
need this support. If we added a constructor like:
> 
>  SpaceAddress

Re: [lldb-dev] Advice on architectures with multiple address spaces

2018-05-17 Thread Zdenek Prikryl via lldb-dev


On 04/19/2018 08:22 PM, Jim Ingham wrote:



On Apr 19, 2018, at 10:54 AM, Greg Clayton  wrote:




On Apr 19, 2018, at 10:35 AM, Jim Ingham  wrote:




On Apr 19, 2018, at 9:44 AM, Greg Clayton via lldb-dev 
 wrote:




On Apr 19, 2018, at 6:51 AM, Zdenek Prikryl via lldb-dev 
 wrote:

Hi lldb developers,

I've been researching using lldb + gdbserver stub that is based on Harvard architecture with 
multiple address spaces (one program, multiple data). The commonly adopted approach is that 
everything is mapped to a single "virtual" address space. The stub reads/writes from/to 
the right memory based on the "virtual" addresses. But I'd like to use real addresses 
with address space id instead. So, I've started looking at what has to be changed.

I've enhanced read/write commands (e.g. memory read --as  ...) and RSP 
protocol (new packet) so that the stub can read/write properly. That wasn't that 
complicated.

It might be nice to add a new RSP protocol packet that asks for the address 
space names/values:

qGetAddressSpaces

which would return something like:

1:text;2:data1,3:data2

or it would return not supported. If we get a valid return value from 
qGetAddressSpaces, then it enables the use of the new packet you added above. 
Else it defaults to using the old memory read functions.



Now I've hit an issue with expressions (LLVMUserExpression.cpp) and local 
variables (DWARFExpressions.cpp). There is a lot of memory read/write functions 
that take just an address argument. Is the only way to go to patch all these 
calls? Has anybody solved it differently?

My quick take is that any APIs that take just a lldb::addr_t would need to take 
something like:

struct SpaceAddress {
static constexpr uint32_t kNoSpace = 0;
lldb::addr_t addr;
uint32_t space;
};


I'm curious why you are suggesting another kind of address, rather than adding 
this functionality to Address?  When you actually go to resolve an Address in a 
target with a process you should have everything you need to know to give it 
the proper space.  Then fixing the expression evaluator (and anything else that 
needs fixing) would be a matter of consistently using Address rather than 
lldb::addr_t.  That seems general goodness, since converting to an lldb::addr_t 
loses information.

If we accept lldb_private::Address in all APIs that take a lldb::addr_t 
currently, then we need to always be able to get to the target in case we need 
to add code to resolve the address everywhere. I am thinking of SpaceAddress as 
an augmented lldb::addr_t instead of a section + offset style address. Also, 
there will be addresses in the code and data that do not exist in actual 
sections. Not saying that you couldn't use lldb_private::Address. I am open to 
suggestions though. So your though it remove all API that take lldb::addr_t and 
use lldb_private::Address everywhere all the time?

It has always bugged me that we have these two ways of specifying addresses.  
Are there many/any places that have to resolve an Address to a real address in 
a process that don't have a Target readily available?  That would surprise me.  
I would much rather centralize on one way than adding a third.

Jim


So, does it make sense to start with lldb::addr_t replacement? In other 
words, replace all instances of lldb::addr_t with Address. It'd be the 
first step and first patch towards to the ability to extend it in the 
future, right?


Zdenek






Jim



We would need a default value for "space" (feel free to rename) that indicates 
the default address space as most of our architectures would not need this support. If we 
added a constructor like:

SpaceAddress(lldb::addr_t a) : addr(a), space(kNoSpace) {}

Then all usages of the APIs that used to take just a "lldb::addr_t" would 
implicitly call this constructor and continue to act as needed. Then we would need to 
allow lldb_private::Address objects to resolve to a SpaceAddress:

SpaceAddress lldb_private::Address::GetSpaceAddress(Target *target) const;

Since each lldb_private::Address has a section and each section knows its 
address space. Then the tricky part is finding all locations in the expression 
parser and converting those to track and use SpaceAddress. We would probably 
need to modify the allocate memory packets in the RSP protocol to be able to 
allocate memory in any address space as well.

I didn't spend much time think about correct names above, so feel free to 
suggest alternate naming.

Best advice:
- make things "just work" to keep changes to a minimum and allowing 
lldb::addr_t to implicitly convert to a SpaceAddress easily
- when modifying RSP, make sure to check for existence of new feature before 
enabling it
- query for address space names so when we dump SpaceAddress we can show 
something that means something to the user. This means we would need to query 
the address space names from the current lldb_private::Process for display.

Submitting might go easier if we break it down into chunks:
1 - ad