Re: [lldb-dev] Symbol Server for everyone.

2016-08-26 Thread Zachary Turner via lldb-dev
By platform agnostic i mean having a single symbol server that works across
multiple platforms is very nice. It sounds like in addition to being a
symbol server this can also serve source code, and should work with
embedded debug info, split dwo, or even pdb?
On Fri, Aug 26, 2016 at 9:54 PM Taras Tsugrii  wrote:

> Zachary, I agree that adding a Python dependency might not be a good idea,
> so I'll take a closer look at the network code available in lldb. Symbol
> format we are currently using is pretty simple - every artifact is
> identified by a type (elf, src, etc), an id (build id for binary or hash
> for source) and a path. I'm not sure what you mean by platform agnostic,
> but with this approach every SymbolVendor will just have to pass the
> appropriate type, build id and a path to a ArtifactManager, which will
> download or locate a locally cached artifact and return a path to it.
> --
> *From:* Zachary Turner 
> *Sent:* Friday, August 26, 2016 8:18:54 PM
> *To:* Taras Tsugrii; lldb-dev@lists.llvm.org
> *Cc:* Kevin Frei
> *Subject:* Re: [lldb-dev] Symbol Server for everyone.
>
> Making the SymbolVendor dependent on Python is probably a non starter, and
> it would also make debugging more difficult.
>
> We have network code for various platforms in Host already.
>
> It would be nice to have a symbol server format that is platform agnostic.
> On the other hand, Microsoft tools already understand their own symbol
> server format , so if i ever reprioritize this, we will probably want the
> standard Microsoft symbol server format on Windows for interoperability.
>
>
> On Fri, Aug 26, 2016 at 8:00 PM Taras Tsugrii via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>> Hello lldb developers,
>>
>>
>> In one of the older posts (
>> http://blog.llvm.org/2015/01/lldb-is-coming-to-windows.html
>> ),
>> symbol server support was mentioned. Most likely it was meant for Windows,
>> but at FB we have our own symbol server implementation for Linux
>> (technically it's completely platform agnostic), which we would like to
>> integrate with LLDB and eventually open source along with the server. As
>> such I thought I'd ask LLDB gurus like you, if anyone is already working on
>> symbol server support and if not, I'd appreciate your thoughts on a desired
>> architecture.
>>
>>
>> General idea.
>>
>> Based on current LLDB implementation and the fact that symbol server
>> feature is a cross-cutting concern, the natural place to put this logic
>> would be inside SymbolVendor plugin, which on Mac is used to resolve
>> separate dSYM bundles. In theory symbol server logic is completely
>> platform-agnostic, as all we need to know is some sort of binary ID (could
>> either be a real .build-id or UUID or some custom logic to compute a stable
>> binary hash) and binary name. This info can be used to make a network
>> request to check whether corresponding binary exists and if so, download it
>> to a temporary location and call symbol_vendor->AddSymbolFileRepresentation
>> with FileSpec pointing at that temporary location.
>>
>>
>> Implementation details.
>>
>>
>> Logic placement.
>>
>> Even though symbol resolution is platform agnostic, the process of
>> extracting/computing binary ID is. As such it seems like
>> SymbolServerResolver can either be a part of LLDB core, or a separate
>> directory in Plugins/SymbolVendor, which will then be used by
>> SymbolVendorELF and SymbolVendorMacOSX. First both symbol vendors will try
>> to resolve the symbols the way they currently do and only if they cannot
>> find anything, will they try to use SymbolVendorSymbolServer.
>>
>> Alternatively symbol server resolution logic can be placed into its own
>> SymbolVendorSymbolServer, and modify SymbolVendor FindPlugin's logic such
>> that it does not return the first found SymbolVendor instance and instead
>> returns either the first SymbolVendor instance that managed to successfully
>> resolve symbols or just last one.
>>
>> Yet another alternative would be to use a delegation chain, such that any
>> SymbolVendor could be wrapped into a SymbolVendorSymbolServer, which would
>> first try to invoke the delegate and if it cannot find symbols, will try to
>> perform its magic. This approach seems nice, but does not play nice with
>> current implementation based on static factory method.
>>
>>
>> Symbol server communication.
>>
>> Network communication can either be implemented natively for different
>> platforms or it can be delegated to a python script invoked by
>> ScriptInterpreter. Using Python seems an easier option in order to make
>> this cross-platform, but it adds a dependency on Python and will require
>> propagating ScriptInterpreter to SymbolVend

Re: [lldb-dev] Symbol Server for everyone.

2016-08-26 Thread Taras Tsugrii via lldb-dev
Zachary, I agree that adding a Python dependency might not be a good idea, so 
I'll take a closer look at the network code available in lldb. Symbol format we 
are currently using is pretty simple - every artifact is identified by a type 
(elf, src, etc), an id (build id for binary or hash for source) and a path. I'm 
not sure what you mean by platform agnostic, but with this approach every 
SymbolVendor will just have to pass the appropriate type, build id and a path 
to a ArtifactManager, which will download or locate a locally cached artifact 
and return a path to it.


From: Zachary Turner 
Sent: Friday, August 26, 2016 8:18:54 PM
To: Taras Tsugrii; lldb-dev@lists.llvm.org
Cc: Kevin Frei
Subject: Re: [lldb-dev] Symbol Server for everyone.

Making the SymbolVendor dependent on Python is probably a non starter, and it 
would also make debugging more difficult.

We have network code for various platforms in Host already.

It would be nice to have a symbol server format that is platform agnostic. On 
the other hand, Microsoft tools already understand their own symbol server 
format , so if i ever reprioritize this, we will probably want the standard 
Microsoft symbol server format on Windows for interoperability.


On Fri, Aug 26, 2016 at 8:00 PM Taras Tsugrii via lldb-dev 
mailto:lldb-dev@lists.llvm.org>> wrote:

Hello lldb developers,


In one of the older posts 
(http://blog.llvm.org/2015/01/lldb-is-coming-to-windows.html),
 symbol server support was mentioned. Most likely it was meant for Windows, but 
at FB we have our own symbol server implementation for Linux (technically it's 
completely platform agnostic), which we would like to integrate with LLDB and 
eventually open source along with the server. As such I thought I'd ask LLDB 
gurus like you, if anyone is already working on symbol server support and if 
not, I'd appreciate your thoughts on a desired architecture.


General idea.

Based on current LLDB implementation and the fact that symbol server feature is 
a cross-cutting concern, the natural place to put this logic would be inside 
SymbolVendor plugin, which on Mac is used to resolve separate dSYM bundles. In 
theory symbol server logic is completely platform-agnostic, as all we need to 
know is some sort of binary ID (could either be a real .build-id or UUID or 
some custom logic to compute a stable binary hash) and binary name. This info 
can be used to make a network request to check whether corresponding binary 
exists and if so, download it to a temporary location and call 
symbol_vendor->AddSymbolFileRepresentation with FileSpec pointing at that 
temporary location.


Implementation details.


Logic placement.

Even though symbol resolution is platform agnostic, the process of 
extracting/computing binary ID is. As such it seems like SymbolServerResolver 
can either be a part of LLDB core, or a separate directory in 
Plugins/SymbolVendor, which will then be used by SymbolVendorELF and 
SymbolVendorMacOSX. First both symbol vendors will try to resolve the symbols 
the way they currently do and only if they cannot find anything, will they try 
to use SymbolVendorSymbolServer.

Alternatively symbol server resolution logic can be placed into its own 
SymbolVendorSymbolServer, and modify SymbolVendor FindPlugin's logic such that 
it does not return the first found SymbolVendor instance and instead returns 
either the first SymbolVendor instance that managed to successfully resolve 
symbols or just last one.

Yet another alternative would be to use a delegation chain, such that any 
SymbolVendor could be wrapped into a SymbolVendorSymbolServer, which would 
first try to invoke the delegate and if it cannot find symbols, will try to 
perform its magic. This approach seems nice, but does not play nice with 
current implementation based on static factory method.


Symbol server communication.

Network communication can either be implemented natively for different 
platforms or it can be delegated to a python script invoked by 
ScriptInterpreter. Using Python seems an easier option in order to make this 
cross-platform, but it adds a dependency on Python and will require propagating 
ScriptInterpreter to SymbolVendor creation factory.


Thoughts, suggestions and comments are very welcome.


Thank you,

Taras

LLVM Project Blog: LLDB is Coming to 
Windows
blog.llvm.org

Re: [lldb-dev] Symbol Server for everyone.

2016-08-26 Thread Zachary Turner via lldb-dev
Making the SymbolVendor dependent on Python is probably a non starter, and
it would also make debugging more difficult.

We have network code for various platforms in Host already.

It would be nice to have a symbol server format that is platform agnostic.
On the other hand, Microsoft tools already understand their own symbol
server format , so if i ever reprioritize this, we will probably want the
standard Microsoft symbol server format on Windows for interoperability.


On Fri, Aug 26, 2016 at 8:00 PM Taras Tsugrii via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> Hello lldb developers,
>
>
> In one of the older posts (
> http://blog.llvm.org/2015/01/lldb-is-coming-to-windows.html), symbol
> server support was mentioned. Most likely it was meant for Windows, but at
> FB we have our own symbol server implementation for Linux (technically it's
> completely platform agnostic), which we would like to integrate with LLDB
> and eventually open source along with the server. As such I thought I'd ask
> LLDB gurus like you, if anyone is already working on symbol server support
> and if not, I'd appreciate your thoughts on a desired architecture.
>
>
> General idea.
>
> Based on current LLDB implementation and the fact that symbol server
> feature is a cross-cutting concern, the natural place to put this logic
> would be inside SymbolVendor plugin, which on Mac is used to resolve
> separate dSYM bundles. In theory symbol server logic is completely
> platform-agnostic, as all we need to know is some sort of binary ID (could
> either be a real .build-id or UUID or some custom logic to compute a stable
> binary hash) and binary name. This info can be used to make a network
> request to check whether corresponding binary exists and if so, download it
> to a temporary location and call symbol_vendor->AddSymbolFileRepresentation
> with FileSpec pointing at that temporary location.
>
>
> Implementation details.
>
>
> Logic placement.
>
> Even though symbol resolution is platform agnostic, the process of
> extracting/computing binary ID is. As such it seems like
> SymbolServerResolver can either be a part of LLDB core, or a separate
> directory in Plugins/SymbolVendor, which will then be used by
> SymbolVendorELF and SymbolVendorMacOSX. First both symbol vendors will try
> to resolve the symbols the way they currently do and only if they cannot
> find anything, will they try to use SymbolVendorSymbolServer.
>
> Alternatively symbol server resolution logic can be placed into its own
> SymbolVendorSymbolServer, and modify SymbolVendor FindPlugin's logic such
> that it does not return the first found SymbolVendor instance and instead
> returns either the first SymbolVendor instance that managed to successfully
> resolve symbols or just last one.
>
> Yet another alternative would be to use a delegation chain, such that any
> SymbolVendor could be wrapped into a SymbolVendorSymbolServer, which would
> first try to invoke the delegate and if it cannot find symbols, will try to
> perform its magic. This approach seems nice, but does not play nice with
> current implementation based on static factory method.
>
>
> Symbol server communication.
>
> Network communication can either be implemented natively for different
> platforms or it can be delegated to a python script invoked by
> ScriptInterpreter. Using Python seems an easier option in order to make
> this cross-platform, but it adds a dependency on Python and will require
> propagating ScriptInterpreter to SymbolVendor creation factory.
>
>
> Thoughts, suggestions and comments are very welcome.
>
>
> Thank you,
>
> Taras
> LLVM Project Blog: LLDB is Coming to Windows
> 
> blog.llvm.org
> This preliminary bootstraping work is mostly complete, and you can use
> LLDB to debug simple executables generated with Clang on Windows today.
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries available

2016-08-26 Thread Dan Walmsley via lldb-dev
Hans,

   Are these new RC3 Windows binaries now with asserts disabled?

Kind Regards

Dan

From: Hans Wennborg via cfe-dev
Sent: 26 August 2016 22:30
To: llvm-dev; 
cfe-dev; LLDB 
Dev; openmp-dev 
(openmp-...@lists.llvm.org)
Cc: Release-testers
Subject: [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries 
available

We're very very close to the final release. Source and binaries for
LLVM-3.9.0-rc3 are available at
http://llvm.org/pre-releases/3.9.0/#rc3

This release candidate is almost the same as rc2, with the following
additional commits:

r279224 - Minor change to OpenCL release notes
r279260 - [lld] Add a note that 3.9 is a major milestone for us
r279468, r279474 - Fix gather-root.ll SLP vectorizer test to not expose UB
r279476 - [lld] Add R_386_TLS_LE as a relocation having an implicit addend.
r279471 - [msan] Disable prlimit test on glibc < 2.13
r279477 - [CloneFunction] Don't remove unrelated nodes from the CGSSC
r279647 - [SCCP] Don't delete side-effecting instructions

We're a little bit behind schedule and there is still an open release
blocker in the Bugzilla, but I'm hopeful that we can wrap up the
release next week.

Thanks,
Hans
___
cfe-dev mailing list
cfe-...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries available

2016-08-26 Thread Dan Walmsley via lldb-dev
Many thanks, I will be giving these a thorough test.

From: Hans Wennborg
Sent: 26 August 2016 23:28
To: Dan Walmsley
Cc: Hans Wennborg via cfe-dev; 
llvm-dev; LLDB 
Dev; openmp-dev 
(openmp-...@lists.llvm.org); 
Release-testers
Subject: Re: [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries 
available

On Fri, Aug 26, 2016 at 3:23 PM, Dan Walmsley via cfe-dev
 wrote:
>Are these new RC3 Windows binaries now with asserts disabled?

Yes.

Thanks,
Hans

> From: Hans Wennborg via cfe-dev
> Sent: 26 August 2016 22:30
> To: llvm-dev; cfe-dev; LLDB Dev; openmp-dev (openmp-...@lists.llvm.org)
> Cc: Release-testers
> Subject: [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries
> available
>
>
>
> We're very very close to the final release. Source and binaries for
> LLVM-3.9.0-rc3 are available at
> http://llvm.org/pre-releases/3.9.0/#rc3
>
> This release candidate is almost the same as rc2, with the following
> additional commits:
>
> r279224 - Minor change to OpenCL release notes
> r279260 - [lld] Add a note that 3.9 is a major milestone for us
> r279468, r279474 - Fix gather-root.ll SLP vectorizer test to not expose UB
> r279476 - [lld] Add R_386_TLS_LE as a relocation having an implicit addend.
> r279471 - [msan] Disable prlimit test on glibc < 2.13
> r279477 - [CloneFunction] Don't remove unrelated nodes from the CGSSC
> r279647 - [SCCP] Don't delete side-effecting instructions
>
> We're a little bit behind schedule and there is still an open release
> blocker in the Bugzilla, but I'm hopeful that we can wrap up the
> release next week.
>
> Thanks,
> Hans
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] Symbol Server for everyone.

2016-08-26 Thread Taras Tsugrii via lldb-dev
Hello lldb developers,


In one of the older posts 
(http://blog.llvm.org/2015/01/lldb-is-coming-to-windows.html), symbol server 
support was mentioned. Most likely it was meant for Windows, but at FB we have 
our own symbol server implementation for Linux (technically it's completely 
platform agnostic), which we would like to integrate with LLDB and eventually 
open source along with the server. As such I thought I'd ask LLDB gurus like 
you, if anyone is already working on symbol server support and if not, I'd 
appreciate your thoughts on a desired architecture.


General idea.

Based on current LLDB implementation and the fact that symbol server feature is 
a cross-cutting concern, the natural place to put this logic would be inside 
SymbolVendor plugin, which on Mac is used to resolve separate dSYM bundles. In 
theory symbol server logic is completely platform-agnostic, as all we need to 
know is some sort of binary ID (could either be a real .build-id or UUID or 
some custom logic to compute a stable binary hash) and binary name. This info 
can be used to make a network request to check whether corresponding binary 
exists and if so, download it to a temporary location and call 
symbol_vendor->AddSymbolFileRepresentation with FileSpec pointing at that 
temporary location.


Implementation details.


Logic placement.

Even though symbol resolution is platform agnostic, the process of 
extracting/computing binary ID is. As such it seems like SymbolServerResolver 
can either be a part of LLDB core, or a separate directory in 
Plugins/SymbolVendor, which will then be used by SymbolVendorELF and 
SymbolVendorMacOSX. First both symbol vendors will try to resolve the symbols 
the way they currently do and only if they cannot find anything, will they try 
to use SymbolVendorSymbolServer.

Alternatively symbol server resolution logic can be placed into its own 
SymbolVendorSymbolServer, and modify SymbolVendor FindPlugin's logic such that 
it does not return the first found SymbolVendor instance and instead returns 
either the first SymbolVendor instance that managed to successfully resolve 
symbols or just last one.

Yet another alternative would be to use a delegation chain, such that any 
SymbolVendor could be wrapped into a SymbolVendorSymbolServer, which would 
first try to invoke the delegate and if it cannot find symbols, will try to 
perform its magic. This approach seems nice, but does not play nice with 
current implementation based on static factory method.


Symbol server communication.

Network communication can either be implemented natively for different 
platforms or it can be delegated to a python script invoked by 
ScriptInterpreter. Using Python seems an easier option in order to make this 
cross-platform, but it adds a dependency on Python and will require propagating 
ScriptInterpreter to SymbolVendor creation factory.


Thoughts, suggestions and comments are very welcome.


Thank you,

Taras

LLVM Project Blog: LLDB is Coming to 
Windows
blog.llvm.org
This preliminary bootstraping work is mostly complete, and you can use LLDB to 
debug simple executables generated with Clang on Windows today.


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution

2016-08-26 Thread Zachary Turner via lldb-dev
Yea, all I did was copy the code and reduce the indent level.  There are
other issues with the code that are non-conformant for LLVM style, but
clang-format can catch all of those.  It can't early indent your code for
you :)

On Fri, Aug 26, 2016 at 6:17 PM Mehdi Amini  wrote:

> On Aug 26, 2016, at 6:12 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
> Back to the formatting issue, there's a lot of code that's going to look
> bad after the reformat, because we have some DEEPLY indented code.  LLVM
> has adopted the early return model for this reason.  A huge amount of our
> deeply nested code could be solved by using early returns.  For example,
> here's some code in a function I was just looking at:
>
> // The 'A' packet is the most over designed packet ever here with
> // redundant argument indexes, redundant argument lengths and needed
> hex
> // encoded argument string values. Really all that is needed is a comma
> // separated hex encoded argument value list, but we will stay true to
> the
> // documented version of the 'A' packet here...
>
> Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
> int actual_arg_index = 0;
>
> packet.SetFilePos(1); // Skip the 'A'
> bool success = true;
> while (success && packet.GetBytesLeft() > 0)
> {
> // Decode the decimal argument string length. This length is the
> // number of hex nibbles in the argument string value.
> const uint32_t arg_len = packet.GetU32(UINT32_MAX);
> if (arg_len == UINT32_MAX)
> success = false;
> else
> {
> // Make sure the argument hex string length is followed by a
> comma
> if (packet.GetChar() != ',')
> success = false;
> else
> {
> // Decode the argument index. We ignore this really because
> // who would really send down the arguments in a random
> order???
> const uint32_t arg_idx = packet.GetU32(UINT32_MAX);
> if (arg_idx == UINT32_MAX)
> success = false;
> else
> {
> // Make sure the argument index is followed by a comma
> if (packet.GetChar() != ',')
> success = false;
> else
> {
> // Decode the argument string value from hex bytes
> // back into a UTF8 string and make sure the length
> // matches the one supplied in the packet
> std::string arg;
> if (packet.GetHexByteStringFixedLength(arg,
> arg_len) != (arg_len / 2))
> success = false;
> else
> {
> // If there are any bytes left
> if (packet.GetBytesLeft())
> {
> if (packet.GetChar() != ',')
> success = false;
> }
>
> if (success)
> {
> if (arg_idx == 0)
>
> m_process_launch_info.GetExecutableFile().SetFile(arg.c_str(), false);
>
> m_process_launch_info.GetArguments().AppendArgument(arg.c_str());
> if (log)
> log->Printf ("LLGSPacketHandler::%s
> added arg %d: \"%s\"", __FUNCTION__, actual_arg_index, arg.c_str ());
> ++actual_arg_index;
> }
> }
> }
> }
> }
> }
> }
>
>
>
> Whether we like early return or not, it is the LLVM Style, and when you
> have to deal with an 80 column wrapping limitation, it definitely helps.
> For example, the above function becomes:
>
>
> Since you’re going with the LLVM style, just a few notes (maybe you
> quickly added the early return without clang-formatting):
>
>
> // The 'A' packet is the most over designed packet ever here with
> // redundant argument indexes, redundant argument lengths and needed
> hex
> // encoded argument string values. Really all that is needed is a comma
> // separated hex encoded argument value list, but we will stay true to
> the
> // documented version of the 'A' packet here...
>
> Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
> int actual_arg_index = 0;
>
> packet.SetFilePos(1); // Skip the 'A'
> while (packet.GetBytesLeft() > 0)
> {
>
>
> Actually I believe the opening bracket here should be on the same line as
> the while.
>
> // Decode the decimal argument string length. This length is the
> // number of hex nibbles in the argument string value.
> const uint32_t arg_len

Re: [lldb-dev] LLDB Evolution

2016-08-26 Thread Mehdi Amini via lldb-dev

> On Aug 26, 2016, at 6:12 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> Back to the formatting issue, there's a lot of code that's going to look bad 
> after the reformat, because we have some DEEPLY indented code.  LLVM has 
> adopted the early return model for this reason.  A huge amount of our deeply 
> nested code could be solved by using early returns.  For example, here's some 
> code in a function I was just looking at:
> 
> // The 'A' packet is the most over designed packet ever here with
> // redundant argument indexes, redundant argument lengths and needed hex
> // encoded argument string values. Really all that is needed is a comma
> // separated hex encoded argument value list, but we will stay true to the
> // documented version of the 'A' packet here...
> 
> Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
> int actual_arg_index = 0;
> 
> packet.SetFilePos(1); // Skip the 'A'
> bool success = true;
> while (success && packet.GetBytesLeft() > 0)
> {
> // Decode the decimal argument string length. This length is the
> // number of hex nibbles in the argument string value.
> const uint32_t arg_len = packet.GetU32(UINT32_MAX);
> if (arg_len == UINT32_MAX)
> success = false;
> else
> {
> // Make sure the argument hex string length is followed by a comma
> if (packet.GetChar() != ',')
> success = false;
> else
> {
> // Decode the argument index. We ignore this really because
> // who would really send down the arguments in a random 
> order???
> const uint32_t arg_idx = packet.GetU32(UINT32_MAX);
> if (arg_idx == UINT32_MAX)
> success = false;
> else
> {
> // Make sure the argument index is followed by a comma
> if (packet.GetChar() != ',')
> success = false;
> else
> {
> // Decode the argument string value from hex bytes
> // back into a UTF8 string and make sure the length
> // matches the one supplied in the packet
> std::string arg;
> if (packet.GetHexByteStringFixedLength(arg, arg_len) 
> != (arg_len / 2))
> success = false;
> else
> {
> // If there are any bytes left
> if (packet.GetBytesLeft())
> {
> if (packet.GetChar() != ',')
> success = false;
> }
> 
> if (success)
> {
> if (arg_idx == 0)
> 
> m_process_launch_info.GetExecutableFile().SetFile(arg.c_str(), false);
> 
> m_process_launch_info.GetArguments().AppendArgument(arg.c_str());
> if (log)
> log->Printf ("LLGSPacketHandler::%s added 
> arg %d: \"%s\"", __FUNCTION__, actual_arg_index, arg.c_str ());
> ++actual_arg_index;
> }
> }
> }
> }
> }
> }
> }
> 
> 
> 
> Whether we like early return or not, it is the LLVM Style, and when you have 
> to deal with an 80 column wrapping limitation, it definitely helps.  For 
> example, the above function becomes:

Since you’re going with the LLVM style, just a few notes (maybe you quickly 
added the early return without clang-formatting): 

> 
> // The 'A' packet is the most over designed packet ever here with
> // redundant argument indexes, redundant argument lengths and needed hex
> // encoded argument string values. Really all that is needed is a comma
> // separated hex encoded argument value list, but we will stay true to the
> // documented version of the 'A' packet here...
> 
> Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
> int actual_arg_index = 0;
> 
> packet.SetFilePos(1); // Skip the 'A'
> while (packet.GetBytesLeft() > 0)
> {

Actually I believe the opening bracket here should be on the same line as the 
while.

> // Decode the decimal argument string length. This length is the
> // number of hex nibbles in the argument string value.
> const uint32_t arg_len = packet.GetU32(UINT32_MAX);
>   if (arg_len == UINT32_MAX)

Any reason the if isn’t on the same indentation as the previous line? (As for 
almost every other if apparently)

>   return false;
>

Re: [lldb-dev] LLDB Evolution

2016-08-26 Thread Zachary Turner via lldb-dev
Back to the formatting issue, there's a lot of code that's going to look
bad after the reformat, because we have some DEEPLY indented code.  LLVM
has adopted the early return model for this reason.  A huge amount of our
deeply nested code could be solved by using early returns.  For example,
here's some code in a function I was just looking at:

// The 'A' packet is the most over designed packet ever here with
// redundant argument indexes, redundant argument lengths and needed hex
// encoded argument string values. Really all that is needed is a comma
// separated hex encoded argument value list, but we will stay true to
the
// documented version of the 'A' packet here...

Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
int actual_arg_index = 0;

packet.SetFilePos(1); // Skip the 'A'
bool success = true;
while (success && packet.GetBytesLeft() > 0)
{
// Decode the decimal argument string length. This length is the
// number of hex nibbles in the argument string value.
const uint32_t arg_len = packet.GetU32(UINT32_MAX);
if (arg_len == UINT32_MAX)
success = false;
else
{
// Make sure the argument hex string length is followed by a
comma
if (packet.GetChar() != ',')
success = false;
else
{
// Decode the argument index. We ignore this really because
// who would really send down the arguments in a random
order???
const uint32_t arg_idx = packet.GetU32(UINT32_MAX);
if (arg_idx == UINT32_MAX)
success = false;
else
{
// Make sure the argument index is followed by a comma
if (packet.GetChar() != ',')
success = false;
else
{
// Decode the argument string value from hex bytes
// back into a UTF8 string and make sure the length
// matches the one supplied in the packet
std::string arg;
if (packet.GetHexByteStringFixedLength(arg,
arg_len) != (arg_len / 2))
success = false;
else
{
// If there are any bytes left
if (packet.GetBytesLeft())
{
if (packet.GetChar() != ',')
success = false;
}

if (success)
{
if (arg_idx == 0)

m_process_launch_info.GetExecutableFile().SetFile(arg.c_str(), false);

m_process_launch_info.GetArguments().AppendArgument(arg.c_str());
if (log)
log->Printf ("LLGSPacketHandler::%s
added arg %d: \"%s\"", __FUNCTION__, actual_arg_index, arg.c_str ());
++actual_arg_index;
}
}
}
}
}
}
}



Whether we like early return or not, it is the LLVM Style, and when you
have to deal with an 80 column wrapping limitation, it definitely helps.
For example, the above function becomes:

// The 'A' packet is the most over designed packet ever here with
// redundant argument indexes, redundant argument lengths and needed hex
// encoded argument string values. Really all that is needed is a comma
// separated hex encoded argument value list, but we will stay true to
the
// documented version of the 'A' packet here...

Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
int actual_arg_index = 0;

packet.SetFilePos(1); // Skip the 'A'
while (packet.GetBytesLeft() > 0)
{
// Decode the decimal argument string length. This length is the
// number of hex nibbles in the argument string value.
const uint32_t arg_len = packet.GetU32(UINT32_MAX);
if (arg_len == UINT32_MAX)
return false;
// Make sure the argument hex string length is followed by a comma
if (packet.GetChar() != ',')
return false;

// Decode the argument index. We ignore this really because
// who would really send down the arguments in a random order???
const uint32_t arg_idx = packet.GetU32(UINT32_MAX);
if (arg_idx == UINT32_MAX)
return false;

// Make sure the argument index is followed by a comma
if (packet.GetChar() != ',')
return false;
// Decode the argument string value from hex bytes
// back into a UTF8 string and make sure the length
// matches the one supplied in the packet
std::string arg;
if (packet.GetHexByteStringFixedLength(arg, arg_len) != (arg_len / 2))
r

Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
How would you enforce that, other than to ask people to try to remember not
to do it?  It seems to me like std::atomic not being copy-constructible is
telling you that, well, you shouldn't be copying it.

BTW, nobody has commented on my original concern that the atomic may not
even be necessary in the first place.

On Fri, Aug 26, 2016 at 5:13 PM Greg Clayton  wrote:

> There is no need to delete the lldb_private::Address copy constructor.
> Just stop functions from passing it by value.
>
> > On Aug 26, 2016, at 1:13 PM, Zachary Turner  wrote:
> >
> > IOW, marking it =delete() is no different than deleting the copy
> constructor above, but at least if you mark it delete, maybe someone will
> read the comment above it that explains why it's deleted :)
> >
> > On Fri, Aug 26, 2016 at 1:13 PM Zachary Turner 
> wrote:
> > I think so.  But in this case lldb::Address explicitly supplied a copy
> constructor that looked like this:
> >
> > Address (const Address& rhs) :
> > m_section_wp (rhs.m_section_wp),
> > m_offset(rhs.m_offset.load())   // this is the std::atomic<>
> > {
> > }
> >
> > circumventing the problem.
> >
> > On Fri, Aug 26, 2016 at 1:11 PM Mehdi Amini 
> wrote:
> >> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >>
> >> It seems to be.  I will also make the copy constructor =delete() to
> make sure this cannot happen again.
> >
> > Just curious: if a member has a deleted copy-ctor (like std::atomic
> right?), isn’t the copy constructor automatically deleted?
> >
> > —
> > Mehdi
> >
> >
> >>
> >> I'm still concerned that the std::atomic is unnecessary, but at that
> point at least it just becomes a performance problem and not a bug.
> >>
> >> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton 
> wrote:
> >> So after speaking with local experts on the subject, we do indeed have
> a problem. Please convert all placed where we pass lldb_private::Address by
> value to pass by "const Address &". Anyone that is modifying the address
> should make a local copy and work with that.
> >>
> >> Is Address the only class that is causing problems?
> >>
> >> Greg
> >>
> >> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >> >
> >> > I recently updated to Visual Studio 2015 Update 3, which has improved
> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
> of errors of the following nature:
> >> >
> >> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
> won't be aligned
> >> >
> >> > The issue comes down to the fact that lldb::Address contains a
> std::atomic, and is being passed by value pervasively throughout
> the codebase.  There is no way to guarantee that this value is 8 byte
> aligned.  This has always been a bug, but until now the compiler just
> hasn't been reporting it.
> >> >
> >> > Someone correct me if I'm wrong, but I believe this is a problem on
> any 32-bit platform, and MSVC is just the only one erroring.
> >> >
> >> > I'm not really sure what to do about this.  Passing
> std::atomic's by value seems wrong to me.
> >> >
> >> > Looking at the code, I don't even know why it needs to be atomic.
> It's not even being used safely.  We'll have a single function write the
> value and later read the value, even though it could have been used in the
> meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't
> need to be atomic in the first place.
> >> >
> >> > Does anyone have a suggestion on what to do about this?  I'm
> currently blocked on this as I can't compile LLDB.
> >> > ___
> >> > lldb-dev mailing list
> >> > lldb-dev@lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> >>
> >> ___
> >> lldb-dev mailing list
> >> lldb-dev@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Greg Clayton via lldb-dev
There is no need to delete the lldb_private::Address copy constructor. Just 
stop functions from passing it by value.

> On Aug 26, 2016, at 1:13 PM, Zachary Turner  wrote:
> 
> IOW, marking it =delete() is no different than deleting the copy constructor 
> above, but at least if you mark it delete, maybe someone will read the 
> comment above it that explains why it's deleted :)
> 
> On Fri, Aug 26, 2016 at 1:13 PM Zachary Turner  wrote:
> I think so.  But in this case lldb::Address explicitly supplied a copy 
> constructor that looked like this:
> 
> Address (const Address& rhs) :
> m_section_wp (rhs.m_section_wp),
> m_offset(rhs.m_offset.load())   // this is the std::atomic<>
> {
> }
> 
> circumventing the problem.
> 
> On Fri, Aug 26, 2016 at 1:11 PM Mehdi Amini  wrote:
>> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev 
>>  wrote:
>> 
>> It seems to be.  I will also make the copy constructor =delete() to make 
>> sure this cannot happen again.
> 
> Just curious: if a member has a deleted copy-ctor (like std::atomic right?), 
> isn’t the copy constructor automatically deleted?
> 
> — 
> Mehdi
> 
> 
>> 
>> I'm still concerned that the std::atomic is unnecessary, but at that point 
>> at least it just becomes a performance problem and not a bug.
>> 
>> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton  wrote:
>> So after speaking with local experts on the subject, we do indeed have a 
>> problem. Please convert all placed where we pass lldb_private::Address by 
>> value to pass by "const Address &". Anyone that is modifying the address 
>> should make a local copy and work with that.
>> 
>> Is Address the only class that is causing problems?
>> 
>> Greg
>> 
>> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
>> >  wrote:
>> >
>> > I recently updated to Visual Studio 2015 Update 3, which has improved its 
>> > diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
>> > errors of the following nature:
>> >
>> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
>> > 'default_stop_addr': formal parameter with requested alignment of 8 won't 
>> > be aligned
>> >
>> > The issue comes down to the fact that lldb::Address contains a 
>> > std::atomic, and is being passed by value pervasively throughout 
>> > the codebase.  There is no way to guarantee that this value is 8 byte 
>> > aligned.  This has always been a bug, but until now the compiler just 
>> > hasn't been reporting it.
>> >
>> > Someone correct me if I'm wrong, but I believe this is a problem on any 
>> > 32-bit platform, and MSVC is just the only one erroring.
>> >
>> > I'm not really sure what to do about this.  Passing std::atomic's 
>> > by value seems wrong to me.
>> >
>> > Looking at the code, I don't even know why it needs to be atomic.  It's 
>> > not even being used safely.  We'll have a single function write the value 
>> > and later read the value, even though it could have been used in the 
>> > meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't 
>> > need to be atomic in the first place.
>> >
>> > Does anyone have a suggestion on what to do about this?  I'm currently 
>> > blocked on this as I can't compile LLDB.
>> > ___
>> > lldb-dev mailing list
>> > lldb-dev@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> 
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries available

2016-08-26 Thread Hans Wennborg via lldb-dev
On Fri, Aug 26, 2016 at 3:23 PM, Dan Walmsley via cfe-dev
 wrote:
>Are these new RC3 Windows binaries now with asserts disabled?

Yes.

Thanks,
Hans

> From: Hans Wennborg via cfe-dev
> Sent: 26 August 2016 22:30
> To: llvm-dev; cfe-dev; LLDB Dev; openmp-dev (openmp-...@lists.llvm.org)
> Cc: Release-testers
> Subject: [cfe-dev] [3.9 Release] Release Candidate 3 source and binaries
> available
>
>
>
> We're very very close to the final release. Source and binaries for
> LLVM-3.9.0-rc3 are available at
> http://llvm.org/pre-releases/3.9.0/#rc3
>
> This release candidate is almost the same as rc2, with the following
> additional commits:
>
> r279224 - Minor change to OpenCL release notes
> r279260 - [lld] Add a note that 3.9 is a major milestone for us
> r279468, r279474 - Fix gather-root.ll SLP vectorizer test to not expose UB
> r279476 - [lld] Add R_386_TLS_LE as a relocation having an implicit addend.
> r279471 - [msan] Disable prlimit test on glibc < 2.13
> r279477 - [CloneFunction] Don't remove unrelated nodes from the CGSSC
> r279647 - [SCCP] Don't delete side-effecting instructions
>
> We're a little bit behind schedule and there is still an open release
> blocker in the Bugzilla, but I'm hopeful that we can wrap up the
> release next week.
>
> Thanks,
> Hans
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] [3.9 Release] Release Candidate 3 source and binaries available

2016-08-26 Thread Hans Wennborg via lldb-dev
We're very very close to the final release. Source and binaries for
LLVM-3.9.0-rc3 are available at
http://llvm.org/pre-releases/3.9.0/#rc3

This release candidate is almost the same as rc2, with the following
additional commits:

r279224 - Minor change to OpenCL release notes
r279260 - [lld] Add a note that 3.9 is a major milestone for us
r279468, r279474 - Fix gather-root.ll SLP vectorizer test to not expose UB
r279476 - [lld] Add R_386_TLS_LE as a relocation having an implicit addend.
r279471 - [msan] Disable prlimit test on glibc < 2.13
r279477 - [CloneFunction] Don't remove unrelated nodes from the CGSSC
r279647 - [SCCP] Don't delete side-effecting instructions

We're a little bit behind schedule and there is still an open release
blocker in the Bugzilla, but I'm hopeful that we can wrap up the
release next week.

Thanks,
Hans
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [3.9 Release] Release Candidate 3 has been tagged

2016-08-26 Thread Hans Wennborg via lldb-dev
On Wed, Aug 24, 2016 at 8:42 PM, Hans Wennborg  wrote:
> Please take this for a spin. If there are no hiccups, the plan is to
> promote this to 'final' on Friday and ship the release early next
> week.

Windows is ready:
12f424c28f22b1c60f531da2f4ba86e5cdd1ca9c  LLVM-3.9.0-rc3-win32.exe
e840f6b729d15cf9b355bd07af3a7b27d97a3bfa  LLVM-3.9.0-rc3-win64.exe

 - Hans
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Mehdi Amini via lldb-dev

> On Aug 26, 2016, at 1:13 PM, Zachary Turner  wrote:
> 
> IOW, marking it =delete() is no different than deleting the copy constructor 
> above, but at least if you mark it delete, maybe someone will read the 
> comment above it that explains why it's deleted :)

Got it, make sense.

Thanks.

— 
Mehdi


> 
> On Fri, Aug 26, 2016 at 1:13 PM Zachary Turner  > wrote:
> I think so.  But in this case lldb::Address explicitly supplied a copy 
> constructor that looked like this:
> 
> Address (const Address& rhs) :
> m_section_wp (rhs.m_section_wp),
> m_offset(rhs.m_offset.load())   // this is the std::atomic<>
> {
> }
> 
> circumventing the problem.
> 
> On Fri, Aug 26, 2016 at 1:11 PM Mehdi Amini  > wrote:
>> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev 
>> mailto:lldb-dev@lists.llvm.org>> wrote:
>> 
>> It seems to be.  I will also make the copy constructor =delete() to make 
>> sure this cannot happen again.
> 
> Just curious: if a member has a deleted copy-ctor (like std::atomic right?), 
> isn’t the copy constructor automatically deleted?
> 
> — 
> Mehdi
> 
> 
>> 
>> I'm still concerned that the std::atomic is unnecessary, but at that point 
>> at least it just becomes a performance problem and not a bug.
>> 
>> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton > > wrote:
>> So after speaking with local experts on the subject, we do indeed have a 
>> problem. Please convert all placed where we pass lldb_private::Address by 
>> value to pass by "const Address &". Anyone that is modifying the address 
>> should make a local copy and work with that.
>> 
>> Is Address the only class that is causing problems?
>> 
>> Greg
>> 
>> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
>> > mailto:lldb-dev@lists.llvm.org>> wrote:
>> >
>> > I recently updated to Visual Studio 2015 Update 3, which has improved its 
>> > diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
>> > errors of the following nature:
>> >
>> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
>> > 'default_stop_addr': formal parameter with requested alignment of 8 won't 
>> > be aligned
>> >
>> > The issue comes down to the fact that lldb::Address contains a 
>> > std::atomic, and is being passed by value pervasively throughout 
>> > the codebase.  There is no way to guarantee that this value is 8 byte 
>> > aligned.  This has always been a bug, but until now the compiler just 
>> > hasn't been reporting it.
>> >
>> > Someone correct me if I'm wrong, but I believe this is a problem on any 
>> > 32-bit platform, and MSVC is just the only one erroring.
>> >
>> > I'm not really sure what to do about this.  Passing std::atomic's 
>> > by value seems wrong to me.
>> >
>> > Looking at the code, I don't even know why it needs to be atomic.  It's 
>> > not even being used safely.  We'll have a single function write the value 
>> > and later read the value, even though it could have been used in the 
>> > meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't 
>> > need to be atomic in the first place.
>> >
>> > Does anyone have a suggestion on what to do about this?  I'm currently 
>> > blocked on this as I can't compile LLDB.
>> > ___
>> > lldb-dev mailing list
>> > lldb-dev@lists.llvm.org 
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
>> > 
>> 
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org 
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
>> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
IOW, marking it =delete() is no different than deleting the copy
constructor above, but at least if you mark it delete, maybe someone will
read the comment above it that explains why it's deleted :)

On Fri, Aug 26, 2016 at 1:13 PM Zachary Turner  wrote:

> I think so.  But in this case lldb::Address explicitly supplied a copy
> constructor that looked like this:
>
> Address (const Address& rhs) :
> m_section_wp (rhs.m_section_wp),
> m_offset(rhs.m_offset.load())   // this is the std::atomic<>
> {
> }
>
> circumventing the problem.
>
> On Fri, Aug 26, 2016 at 1:11 PM Mehdi Amini  wrote:
>
>> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>>
>> It seems to be.  I will also make the copy constructor =delete() to make
>> sure this cannot happen again.
>>
>>
>> Just curious: if a member has a deleted copy-ctor (like std::atomic
>> right?), isn’t the copy constructor automatically deleted?
>>
>> —
>> Mehdi
>>
>>
>>
>> I'm still concerned that the std::atomic is unnecessary, but at that
>> point at least it just becomes a performance problem and not a bug.
>>
>> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton  wrote:
>>
>>> So after speaking with local experts on the subject, we do indeed have a
>>> problem. Please convert all placed where we pass lldb_private::Address by
>>> value to pass by "const Address &". Anyone that is modifying the address
>>> should make a local copy and work with that.
>>>
>>> Is Address the only class that is causing problems?
>>>
>>> Greg
>>>
>>> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
>>> lldb-dev@lists.llvm.org> wrote:
>>> >
>>> > I recently updated to Visual Studio 2015 Update 3, which has improved
>>> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
>>> of errors of the following nature:
>>> >
>>> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
>>> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
>>> won't be aligned
>>> >
>>> > The issue comes down to the fact that lldb::Address contains a
>>> std::atomic, and is being passed by value pervasively throughout
>>> the codebase.  There is no way to guarantee that this value is 8 byte
>>> aligned.  This has always been a bug, but until now the compiler just
>>> hasn't been reporting it.
>>> >
>>> > Someone correct me if I'm wrong, but I believe this is a problem on
>>> any 32-bit platform, and MSVC is just the only one erroring.
>>> >
>>> > I'm not really sure what to do about this.  Passing
>>> std::atomic's by value seems wrong to me.
>>> >
>>> > Looking at the code, I don't even know why it needs to be atomic.
>>> It's not even being used safely.  We'll have a single function write the
>>> value and later read the value, even though it could have been used in the
>>> meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't
>>> need to be atomic in the first place.
>>> >
>>> > Does anyone have a suggestion on what to do about this?  I'm currently
>>> blocked on this as I can't compile LLDB.
>>> > ___
>>> > lldb-dev mailing list
>>> > lldb-dev@lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>>
>>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>
>>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
I think so.  But in this case lldb::Address explicitly supplied a copy
constructor that looked like this:

Address (const Address& rhs) :
m_section_wp (rhs.m_section_wp),
m_offset(rhs.m_offset.load())   // this is the std::atomic<>
{
}

circumventing the problem.

On Fri, Aug 26, 2016 at 1:11 PM Mehdi Amini  wrote:

> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
> It seems to be.  I will also make the copy constructor =delete() to make
> sure this cannot happen again.
>
>
> Just curious: if a member has a deleted copy-ctor (like std::atomic
> right?), isn’t the copy constructor automatically deleted?
>
> —
> Mehdi
>
>
>
> I'm still concerned that the std::atomic is unnecessary, but at that point
> at least it just becomes a performance problem and not a bug.
>
> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton  wrote:
>
>> So after speaking with local experts on the subject, we do indeed have a
>> problem. Please convert all placed where we pass lldb_private::Address by
>> value to pass by "const Address &". Anyone that is modifying the address
>> should make a local copy and work with that.
>>
>> Is Address the only class that is causing problems?
>>
>> Greg
>>
>> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> >
>> > I recently updated to Visual Studio 2015 Update 3, which has improved
>> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
>> of errors of the following nature:
>> >
>> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
>> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
>> won't be aligned
>> >
>> > The issue comes down to the fact that lldb::Address contains a
>> std::atomic, and is being passed by value pervasively throughout
>> the codebase.  There is no way to guarantee that this value is 8 byte
>> aligned.  This has always been a bug, but until now the compiler just
>> hasn't been reporting it.
>> >
>> > Someone correct me if I'm wrong, but I believe this is a problem on any
>> 32-bit platform, and MSVC is just the only one erroring.
>> >
>> > I'm not really sure what to do about this.  Passing
>> std::atomic's by value seems wrong to me.
>> >
>> > Looking at the code, I don't even know why it needs to be atomic.  It's
>> not even being used safely.  We'll have a single function write the value
>> and later read the value, even though it could have been used in the
>> meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't
>> need to be atomic in the first place.
>> >
>> > Does anyone have a suggestion on what to do about this?  I'm currently
>> blocked on this as I can't compile LLDB.
>> > ___
>> > lldb-dev mailing list
>> > lldb-dev@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>
>> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Mehdi Amini via lldb-dev

> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> It seems to be.  I will also make the copy constructor =delete() to make sure 
> this cannot happen again.

Just curious: if a member has a deleted copy-ctor (like std::atomic right?), 
isn’t the copy constructor automatically deleted?

— 
Mehdi


> 
> I'm still concerned that the std::atomic is unnecessary, but at that point at 
> least it just becomes a performance problem and not a bug.
> 
> On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton  > wrote:
> So after speaking with local experts on the subject, we do indeed have a 
> problem. Please convert all placed where we pass lldb_private::Address by 
> value to pass by "const Address &". Anyone that is modifying the address 
> should make a local copy and work with that.
> 
> Is Address the only class that is causing problems?
> 
> Greg
> 
> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
> > mailto:lldb-dev@lists.llvm.org>> wrote:
> >
> > I recently updated to Visual Studio 2015 Update 3, which has improved its 
> > diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
> > errors of the following nature:
> >
> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> > 'default_stop_addr': formal parameter with requested alignment of 8 won't 
> > be aligned
> >
> > The issue comes down to the fact that lldb::Address contains a 
> > std::atomic, and is being passed by value pervasively throughout 
> > the codebase.  There is no way to guarantee that this value is 8 byte 
> > aligned.  This has always been a bug, but until now the compiler just 
> > hasn't been reporting it.
> >
> > Someone correct me if I'm wrong, but I believe this is a problem on any 
> > 32-bit platform, and MSVC is just the only one erroring.
> >
> > I'm not really sure what to do about this.  Passing std::atomic's 
> > by value seems wrong to me.
> >
> > Looking at the code, I don't even know why it needs to be atomic.  It's not 
> > even being used safely.  We'll have a single function write the value and 
> > later read the value, even though it could have been used in the meantime.  
> > Maybe what is really intended is a mutex.  Or maybe it doesn't need to be 
> > atomic in the first place.
> >
> > Does anyone have a suggestion on what to do about this?  I'm currently 
> > blocked on this as I can't compile LLDB.
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org 
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
> > 
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
It seems to be.  I will also make the copy constructor =delete() to make
sure this cannot happen again.

I'm still concerned that the std::atomic is unnecessary, but at that point
at least it just becomes a performance problem and not a bug.

On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton  wrote:

> So after speaking with local experts on the subject, we do indeed have a
> problem. Please convert all placed where we pass lldb_private::Address by
> value to pass by "const Address &". Anyone that is modifying the address
> should make a local copy and work with that.
>
> Is Address the only class that is causing problems?
>
> Greg
>
> > On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > I recently updated to Visual Studio 2015 Update 3, which has improved
> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
> of errors of the following nature:
> >
> > D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719:
> 'default_stop_addr': formal parameter with requested alignment of 8 won't
> be aligned
> >
> > The issue comes down to the fact that lldb::Address contains a
> std::atomic, and is being passed by value pervasively throughout
> the codebase.  There is no way to guarantee that this value is 8 byte
> aligned.  This has always been a bug, but until now the compiler just
> hasn't been reporting it.
> >
> > Someone correct me if I'm wrong, but I believe this is a problem on any
> 32-bit platform, and MSVC is just the only one erroring.
> >
> > I'm not really sure what to do about this.  Passing
> std::atomic's by value seems wrong to me.
> >
> > Looking at the code, I don't even know why it needs to be atomic.  It's
> not even being used safely.  We'll have a single function write the value
> and later read the value, even though it could have been used in the
> meantime.  Maybe what is really intended is a mutex.  Or maybe it doesn't
> need to be atomic in the first place.
> >
> > Does anyone have a suggestion on what to do about this?  I'm currently
> blocked on this as I can't compile LLDB.
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Greg Clayton via lldb-dev
So after speaking with local experts on the subject, we do indeed have a 
problem. Please convert all placed where we pass lldb_private::Address by value 
to pass by "const Address &". Anyone that is modifying the address should make 
a local copy and work with that.

Is Address the only class that is causing problems?

Greg 

> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
>  wrote:
> 
> I recently updated to Visual Studio 2015 Update 3, which has improved its 
> diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
> errors of the following nature:
> 
> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> 'default_stop_addr': formal parameter with requested alignment of 8 won't be 
> aligned
> 
> The issue comes down to the fact that lldb::Address contains a 
> std::atomic, and is being passed by value pervasively throughout 
> the codebase.  There is no way to guarantee that this value is 8 byte 
> aligned.  This has always been a bug, but until now the compiler just hasn't 
> been reporting it.
> 
> Someone correct me if I'm wrong, but I believe this is a problem on any 
> 32-bit platform, and MSVC is just the only one erroring.
> 
> I'm not really sure what to do about this.  Passing std::atomic's by 
> value seems wrong to me.
> 
> Looking at the code, I don't even know why it needs to be atomic.  It's not 
> even being used safely.  We'll have a single function write the value and 
> later read the value, even though it could have been used in the meantime.  
> Maybe what is really intended is a mutex.  Or maybe it doesn't need to be 
> atomic in the first place.
> 
> Does anyone have a suggestion on what to do about this?  I'm currently 
> blocked on this as I can't compile LLDB.
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
Incidentally, it seems copy and assignment of std::atomics are deleted
functions, so the C++ standard is telling you you're not really supposed to
do this.

I honestly don't believe this should be a std::atomic at all.

On Fri, Aug 26, 2016 at 12:54 PM Ted Woodward via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

>
> Would the misalignment really be a problem on x86? I thought the core
> handled misaligned loads and stores. Unlike, say, PPC.
>
> Either way, I'd expect the compiler to automatically align a uint64.
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
> -Original Message-
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of Jim
> Ingham via lldb-dev
> Sent: Friday, August 26, 2016 2:32 PM
> To: Zachary Turner 
> Cc: LLDB 
> Subject: Re: [lldb-dev] Passing std::atomics by value
>
> That seems to me a compiler bug, not: "You can't pass structures with
> std::atomic" elements in them by value.  It would crazy to add a type to
> the C++ language that you couldn't use everywhere.  There doesn't seem to
> be any official restriction.  And anecdotally there's a reference to the
> problem in gcc for PPC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62259)
> but that was considered a bug and fixed.
>
> Not that it matters much, you are stuck with the compiler you're stuck
> with, bugs and all.
>
> Jim
>
> > On Aug 26, 2016, at 12:26 PM, Zachary Turner  wrote:
> >
> > It could, in theory, it just doesn't.  I compiled a quick program using
> i686-darwin as the target and the generated assembly makes no attempt to
> pad the arguments.
> >
> > I'll post some code later
> >
> > On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham  wrote:
> >
> > > On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >
> > > The thing is, the code is already full of data races.  I think the
> std::atomic is actually used incorrectly and is not even doing anything.
> > >
> > > That said, consider darwin on 32-bit, where I believe each stack frame
> is 4-byte aligned.  I dont' think there's any way the compiler can
> guarantee that a function parameter is 8-byte aligned without allocating
> from the heap, which is obviously impossible for a stack variable.
> >
> > Why can't the compiler pad the argument slot on the stack so that the
> actual struct lives at a properly aligned location?  It's copying the value
> into the callee's stack frame, so it can put it wherever it wants.  And
> both caller and callee sites know the alignment requirements from the
> function signature, so they can both figure out where the actual struct
> lives in the argument slot based on the alignment of the stack slot.
> >
> > Jim
> >
> >
> > >
> > > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton 
> wrote:
> > >
> > > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > > >
> > > >>
> > > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > > >>
> > > >> I recently updated to Visual Studio 2015 Update 3, which has
> improved its diagnostics.  As a result of this, LLDB is uncompilable due to
> a slew of errors of the following nature:
> > > >>
> > > >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
> won't be aligned
> > > >>
> > > >> The issue comes down to the fact that lldb::Address contains a
> std::atomic, and is being passed by value pervasively throughout
> the codebase.  There is no way to guarantee that this value is 8 byte
> aligned.  This has always been a bug, but until now the compiler just
> hasn't been reporting it.
> > > >>
> > > >> Someone correct me if I'm wrong, but I believe this is a problem on
> any 32-bit platform, and MSVC is just the only one erroring.
> > > >>
> > > >> I'm not really sure what to do about this.  Passing
> std::atomic's by value seems wrong to me.
> > > >>
> > > >> Looking at the code, I don't even know why it needs to be atomic.
> It's not even being used safely.  We'll have a single function write the
> value and later read the value, even though it could have been used in the
> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't
> need to be atomic in the first place.
> > > >>
> > > >> Does anyone have a suggestion on what to do about this?  I'm
> currently blocked on this as I can't compile LLDB.
> > > >
> > > > Feel free to #ifdef around the m_offset member of Address and you
> can have the data race and compile. This seems like a compiler bug to me.
> If a struct/classe by value argument has alignment requirements, then the
> compiler should handle this correctly IMHO. Am I wrong
> > >
> > > Or if this isn't a compiler bug, feel free to modify anything that was
> passing Address by value and make it a "const Address &".
> > > __

Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Ted Woodward via lldb-dev

Would the misalignment really be a problem on x86? I thought the core handled 
misaligned loads and stores. Unlike, say, PPC.

Either way, I'd expect the compiler to automatically align a uint64.

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


-Original Message-
From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of Jim Ingham 
via lldb-dev
Sent: Friday, August 26, 2016 2:32 PM
To: Zachary Turner 
Cc: LLDB 
Subject: Re: [lldb-dev] Passing std::atomics by value

That seems to me a compiler bug, not: "You can't pass structures with 
std::atomic" elements in them by value.  It would crazy to add a type to the 
C++ language that you couldn't use everywhere.  There doesn't seem to be any 
official restriction.  And anecdotally there's a reference to the problem in 
gcc for PPC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62259) but that was 
considered a bug and fixed.

Not that it matters much, you are stuck with the compiler you're stuck with, 
bugs and all.

Jim

> On Aug 26, 2016, at 12:26 PM, Zachary Turner  wrote:
> 
> It could, in theory, it just doesn't.  I compiled a quick program using 
> i686-darwin as the target and the generated assembly makes no attempt to pad 
> the arguments.
> 
> I'll post some code later
> 
> On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham  wrote:
> 
> > On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> > The thing is, the code is already full of data races.  I think the 
> > std::atomic is actually used incorrectly and is not even doing anything.
> >
> > That said, consider darwin on 32-bit, where I believe each stack frame is 
> > 4-byte aligned.  I dont' think there's any way the compiler can guarantee 
> > that a function parameter is 8-byte aligned without allocating from the 
> > heap, which is obviously impossible for a stack variable.
> 
> Why can't the compiler pad the argument slot on the stack so that the actual 
> struct lives at a properly aligned location?  It's copying the value into the 
> callee's stack frame, so it can put it wherever it wants.  And both caller 
> and callee sites know the alignment requirements from the function signature, 
> so they can both figure out where the actual struct lives in the argument 
> slot based on the alignment of the stack slot.
> 
> Jim
> 
> 
> >
> > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:
> >
> > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev 
> > >  wrote:
> > >
> > >>
> > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
> > >>  wrote:
> > >>
> > >> I recently updated to Visual Studio 2015 Update 3, which has improved 
> > >> its diagnostics.  As a result of this, LLDB is uncompilable due to a 
> > >> slew of errors of the following nature:
> > >>
> > >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> > >> 'default_stop_addr': formal parameter with requested alignment of 8 
> > >> won't be aligned
> > >>
> > >> The issue comes down to the fact that lldb::Address contains a 
> > >> std::atomic, and is being passed by value pervasively 
> > >> throughout the codebase.  There is no way to guarantee that this value 
> > >> is 8 byte aligned.  This has always been a bug, but until now the 
> > >> compiler just hasn't been reporting it.
> > >>
> > >> Someone correct me if I'm wrong, but I believe this is a problem on any 
> > >> 32-bit platform, and MSVC is just the only one erroring.
> > >>
> > >> I'm not really sure what to do about this.  Passing 
> > >> std::atomic's by value seems wrong to me.
> > >>
> > >> Looking at the code, I don't even know why it needs to be atomic.  It's 
> > >> not even being used safely.  We'll have a single function write the 
> > >> value and later read the value, even though it could have been used in 
> > >> the meantime. Maybe what is really intended is a mutex.  Or maybe it 
> > >> doesn't need to be atomic in the first place.
> > >>
> > >> Does anyone have a suggestion on what to do about this?  I'm currently 
> > >> blocked on this as I can't compile LLDB.
> > >
> > > Feel free to #ifdef around the m_offset member of Address and you can 
> > > have the data race and compile. This seems like a compiler bug to me. If 
> > > a struct/classe by value argument has alignment requirements, then the 
> > > compiler should handle this correctly IMHO. Am I wrong
> >
> > Or if this isn't a compiler bug, feel free to modify anything that was 
> > passing Address by value and make it a "const Address &".
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing 

Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Jim Ingham via lldb-dev
That seems to me a compiler bug, not: "You can't pass structures with 
std::atomic" elements in them by value.  It would crazy to add a type to the 
C++ language that you couldn't use everywhere.  There doesn't seem to be any 
official restriction.  And anecdotally there's a reference to the problem in 
gcc for PPC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62259) but that was 
considered a bug and fixed.

Not that it matters much, you are stuck with the compiler you're stuck with, 
bugs and all.

Jim

> On Aug 26, 2016, at 12:26 PM, Zachary Turner  wrote:
> 
> It could, in theory, it just doesn't.  I compiled a quick program using 
> i686-darwin as the target and the generated assembly makes no attempt to pad 
> the arguments.
> 
> I'll post some code later
> 
> On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham  wrote:
> 
> > On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> > The thing is, the code is already full of data races.  I think the 
> > std::atomic is actually used incorrectly and is not even doing anything.
> >
> > That said, consider darwin on 32-bit, where I believe each stack frame is 
> > 4-byte aligned.  I dont' think there's any way the compiler can guarantee 
> > that a function parameter is 8-byte aligned without allocating from the 
> > heap, which is obviously impossible for a stack variable.
> 
> Why can't the compiler pad the argument slot on the stack so that the actual 
> struct lives at a properly aligned location?  It's copying the value into the 
> callee's stack frame, so it can put it wherever it wants.  And both caller 
> and callee sites know the alignment requirements from the function signature, 
> so they can both figure out where the actual struct lives in the argument 
> slot based on the alignment of the stack slot.
> 
> Jim
> 
> 
> >
> > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:
> >
> > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev 
> > >  wrote:
> > >
> > >>
> > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
> > >>  wrote:
> > >>
> > >> I recently updated to Visual Studio 2015 Update 3, which has improved 
> > >> its diagnostics.  As a result of this, LLDB is uncompilable due to a 
> > >> slew of errors of the following nature:
> > >>
> > >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> > >> 'default_stop_addr': formal parameter with requested alignment of 8 
> > >> won't be aligned
> > >>
> > >> The issue comes down to the fact that lldb::Address contains a 
> > >> std::atomic, and is being passed by value pervasively 
> > >> throughout the codebase.  There is no way to guarantee that this value 
> > >> is 8 byte aligned.  This has always been a bug, but until now the 
> > >> compiler just hasn't been reporting it.
> > >>
> > >> Someone correct me if I'm wrong, but I believe this is a problem on any 
> > >> 32-bit platform, and MSVC is just the only one erroring.
> > >>
> > >> I'm not really sure what to do about this.  Passing 
> > >> std::atomic's by value seems wrong to me.
> > >>
> > >> Looking at the code, I don't even know why it needs to be atomic.  It's 
> > >> not even being used safely.  We'll have a single function write the 
> > >> value and later read the value, even though it could have been used in 
> > >> the meantime. Maybe what is really intended is a mutex.  Or maybe it 
> > >> doesn't need to be atomic in the first place.
> > >>
> > >> Does anyone have a suggestion on what to do about this?  I'm currently 
> > >> blocked on this as I can't compile LLDB.
> > >
> > > Feel free to #ifdef around the m_offset member of Address and you can 
> > > have the data race and compile. This seems like a compiler bug to me. If 
> > > a struct/classe by value argument has alignment requirements, then the 
> > > compiler should handle this correctly IMHO. Am I wrong
> >
> > Or if this isn't a compiler bug, feel free to modify anything that was 
> > passing Address by value and make it a "const Address &".
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
It could, in theory, it just doesn't. I compiled a quick program using
i686-darwin as the target and the generated assembly makes no attempt to
pad the arguments.

I'll post some code later

On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham  wrote:

>
> > On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > The thing is, the code is already full of data races.  I think the
> std::atomic is actually used incorrectly and is not even doing anything.
> >
> > That said, consider darwin on 32-bit, where I believe each stack frame
> is 4-byte aligned.  I dont' think there's any way the compiler can
> guarantee that a function parameter is 8-byte aligned without allocating
> from the heap, which is obviously impossible for a stack variable.
>
> Why can't the compiler pad the argument slot on the stack so that the
> actual struct lives at a properly aligned location?  It's copying the value
> into the callee's stack frame, so it can put it wherever it wants.  And
> both caller and callee sites know the alignment requirements from the
> function signature, so they can both figure out where the actual struct
> lives in the argument slot based on the alignment of the stack slot.
>
> Jim
>
>
> >
> > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton 
> wrote:
> >
> > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >
> > >>
> > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >>
> > >> I recently updated to Visual Studio 2015 Update 3, which has improved
> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
> of errors of the following nature:
> > >>
> > >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
> won't be aligned
> > >>
> > >> The issue comes down to the fact that lldb::Address contains a
> std::atomic, and is being passed by value pervasively throughout
> the codebase.  There is no way to guarantee that this value is 8 byte
> aligned.  This has always been a bug, but until now the compiler just
> hasn't been reporting it.
> > >>
> > >> Someone correct me if I'm wrong, but I believe this is a problem on
> any 32-bit platform, and MSVC is just the only one erroring.
> > >>
> > >> I'm not really sure what to do about this.  Passing
> std::atomic's by value seems wrong to me.
> > >>
> > >> Looking at the code, I don't even know why it needs to be atomic.
> It's not even being used safely.  We'll have a single function write the
> value and later read the value, even though it could have been used in the
> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't
> need to be atomic in the first place.
> > >>
> > >> Does anyone have a suggestion on what to do about this?  I'm
> currently blocked on this as I can't compile LLDB.
> > >
> > > Feel free to #ifdef around the m_offset member of Address and you can
> have the data race and compile. This seems like a compiler bug to me. If a
> struct/classe by value argument has alignment requirements, then the
> compiler should handle this correctly IMHO. Am I wrong
> >
> > Or if this isn't a compiler bug, feel free to modify anything that was
> passing Address by value and make it a "const Address &".
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Jim Ingham via lldb-dev

> On Aug 26, 2016, at 11:44 AM, Adrian McCarthy via lldb-dev 
>  wrote:
> 
> Methods like Address::SetOffset and Address::Slide seem to have data races 
> despite m_offset being atomic.  Callers of those methods would have to 
> guarantee that nothing else is trying to write to m_offset.  And if they're 
> doing that, then there doesn't appear to be a need for std::atomic on that 
> field.
> 
> BTW, I propose we move the member variables from protected to private.  As 
> far as I can tell, there aren't any derived classes (yet), at least none that 
> access those members.  If we need to add a mutex to the class itself, it'll 
> be better if any future derived classes access the data through accessors.

That seems fine to me.  We haven't needed to subclass it in years, and probably 
won't.

Jim


> 
> On Fri, Aug 26, 2016 at 11:36 AM, Zachary Turner via lldb-dev 
>  wrote:
> The thing is, the code is already full of data races.  I think the 
> std::atomic is actually used incorrectly and is not even doing anything.
> 
> That said, consider darwin on 32-bit, where I believe each stack frame is 
> 4-byte aligned.  I dont' think there's any way the compiler can guarantee 
> that a function parameter is 8-byte aligned without allocating from the heap, 
> which is obviously impossible for a stack variable.
> 
> On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:
> 
> > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev 
> >  wrote:
> >
> >>
> >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
> >>  wrote:
> >>
> >> I recently updated to Visual Studio 2015 Update 3, which has improved its 
> >> diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
> >> errors of the following nature:
> >>
> >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> >> 'default_stop_addr': formal parameter with requested alignment of 8 won't 
> >> be aligned
> >>
> >> The issue comes down to the fact that lldb::Address contains a 
> >> std::atomic, and is being passed by value pervasively throughout 
> >> the codebase.  There is no way to guarantee that this value is 8 byte 
> >> aligned.  This has always been a bug, but until now the compiler just 
> >> hasn't been reporting it.
> >>
> >> Someone correct me if I'm wrong, but I believe this is a problem on any 
> >> 32-bit platform, and MSVC is just the only one erroring.
> >>
> >> I'm not really sure what to do about this.  Passing std::atomic's 
> >> by value seems wrong to me.
> >>
> >> Looking at the code, I don't even know why it needs to be atomic.  It's 
> >> not even being used safely.  We'll have a single function write the value 
> >> and later read the value, even though it could have been used in the 
> >> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't 
> >> need to be atomic in the first place.
> >>
> >> Does anyone have a suggestion on what to do about this?  I'm currently 
> >> blocked on this as I can't compile LLDB.
> >
> > Feel free to #ifdef around the m_offset member of Address and you can have 
> > the data race and compile. This seems like a compiler bug to me. If a 
> > struct/classe by value argument has alignment requirements, then the 
> > compiler should handle this correctly IMHO. Am I wrong
> 
> Or if this isn't a compiler bug, feel free to modify anything that was 
> passing Address by value and make it a "const Address &".
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Adrian McCarthy via lldb-dev
Methods like Address::SetOffset and Address::Slide seem to have data races
despite m_offset being atomic.  Callers of those methods would have to
guarantee that nothing else is trying to write to m_offset.  And if they're
doing that, then there doesn't appear to be a need for std::atomic on that
field.

BTW, I propose we move the member variables from protected to private.  As
far as I can tell, there aren't any derived classes (yet), at least none
that access those members.  If we need to add a mutex to the class itself,
it'll be better if any future derived classes access the data through
accessors.

On Fri, Aug 26, 2016 at 11:36 AM, Zachary Turner via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> The thing is, the code is already full of data races.  I think the
> std::atomic is actually used incorrectly and is not even doing anything.
>
> That said, consider darwin on 32-bit, where I believe each stack frame is
> 4-byte aligned.  I dont' think there's any way the compiler can guarantee
> that a function parameter is 8-byte aligned without allocating from the
> heap, which is obviously impossible for a stack variable.
>
> On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:
>
>>
>> > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> >
>> >>
>> >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> >>
>> >> I recently updated to Visual Studio 2015 Update 3, which has improved
>> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
>> of errors of the following nature:
>> >>
>> >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
>> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
>> won't be aligned
>> >>
>> >> The issue comes down to the fact that lldb::Address contains a
>> std::atomic, and is being passed by value pervasively throughout
>> the codebase.  There is no way to guarantee that this value is 8 byte
>> aligned.  This has always been a bug, but until now the compiler just
>> hasn't been reporting it.
>> >>
>> >> Someone correct me if I'm wrong, but I believe this is a problem on
>> any 32-bit platform, and MSVC is just the only one erroring.
>> >>
>> >> I'm not really sure what to do about this.  Passing
>> std::atomic's by value seems wrong to me.
>> >>
>> >> Looking at the code, I don't even know why it needs to be atomic.
>> It's not even being used safely.  We'll have a single function write the
>> value and later read the value, even though it could have been used in the
>> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't
>> need to be atomic in the first place.
>> >>
>> >> Does anyone have a suggestion on what to do about this?  I'm currently
>> blocked on this as I can't compile LLDB.
>> >
>> > Feel free to #ifdef around the m_offset member of Address and you can
>> have the data race and compile. This seems like a compiler bug to me. If a
>> struct/classe by value argument has alignment requirements, then the
>> compiler should handle this correctly IMHO. Am I wrong
>>
>> Or if this isn't a compiler bug, feel free to modify anything that was
>> passing Address by value and make it a "const Address &".
>
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Jim Ingham via lldb-dev

> On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev 
>  wrote:
> 
> The thing is, the code is already full of data races.  I think the 
> std::atomic is actually used incorrectly and is not even doing anything.
> 
> That said, consider darwin on 32-bit, where I believe each stack frame is 
> 4-byte aligned.  I dont' think there's any way the compiler can guarantee 
> that a function parameter is 8-byte aligned without allocating from the heap, 
> which is obviously impossible for a stack variable.

Why can't the compiler pad the argument slot on the stack so that the actual 
struct lives at a properly aligned location?  It's copying the value into the 
callee's stack frame, so it can put it wherever it wants.  And both caller and 
callee sites know the alignment requirements from the function signature, so 
they can both figure out where the actual struct lives in the argument slot 
based on the alignment of the stack slot.

Jim


> 
> On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:
> 
> > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev 
> >  wrote:
> >
> >>
> >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
> >>  wrote:
> >>
> >> I recently updated to Visual Studio 2015 Update 3, which has improved its 
> >> diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
> >> errors of the following nature:
> >>
> >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> >> 'default_stop_addr': formal parameter with requested alignment of 8 won't 
> >> be aligned
> >>
> >> The issue comes down to the fact that lldb::Address contains a 
> >> std::atomic, and is being passed by value pervasively throughout 
> >> the codebase.  There is no way to guarantee that this value is 8 byte 
> >> aligned.  This has always been a bug, but until now the compiler just 
> >> hasn't been reporting it.
> >>
> >> Someone correct me if I'm wrong, but I believe this is a problem on any 
> >> 32-bit platform, and MSVC is just the only one erroring.
> >>
> >> I'm not really sure what to do about this.  Passing std::atomic's 
> >> by value seems wrong to me.
> >>
> >> Looking at the code, I don't even know why it needs to be atomic.  It's 
> >> not even being used safely.  We'll have a single function write the value 
> >> and later read the value, even though it could have been used in the 
> >> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't 
> >> need to be atomic in the first place.
> >>
> >> Does anyone have a suggestion on what to do about this?  I'm currently 
> >> blocked on this as I can't compile LLDB.
> >
> > Feel free to #ifdef around the m_offset member of Address and you can have 
> > the data race and compile. This seems like a compiler bug to me. If a 
> > struct/classe by value argument has alignment requirements, then the 
> > compiler should handle this correctly IMHO. Am I wrong
> 
> Or if this isn't a compiler bug, feel free to modify anything that was 
> passing Address by value and make it a "const Address &".
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
The thing is, the code is already full of data races.  I think the
std::atomic is actually used incorrectly and is not even doing anything.

That said, consider darwin on 32-bit, where I believe each stack frame is
4-byte aligned.  I dont' think there's any way the compiler can guarantee
that a function parameter is 8-byte aligned without allocating from the
heap, which is obviously impossible for a stack variable.

On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton  wrote:

>
> > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> >>
> >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >>
> >> I recently updated to Visual Studio 2015 Update 3, which has improved
> its diagnostics.  As a result of this, LLDB is uncompilable due to a slew
> of errors of the following nature:
> >>
> >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error
> C2719: 'default_stop_addr': formal parameter with requested alignment of 8
> won't be aligned
> >>
> >> The issue comes down to the fact that lldb::Address contains a
> std::atomic, and is being passed by value pervasively throughout
> the codebase.  There is no way to guarantee that this value is 8 byte
> aligned.  This has always been a bug, but until now the compiler just
> hasn't been reporting it.
> >>
> >> Someone correct me if I'm wrong, but I believe this is a problem on any
> 32-bit platform, and MSVC is just the only one erroring.
> >>
> >> I'm not really sure what to do about this.  Passing
> std::atomic's by value seems wrong to me.
> >>
> >> Looking at the code, I don't even know why it needs to be atomic.  It's
> not even being used safely.  We'll have a single function write the value
> and later read the value, even though it could have been used in the
> meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't
> need to be atomic in the first place.
> >>
> >> Does anyone have a suggestion on what to do about this?  I'm currently
> blocked on this as I can't compile LLDB.
> >
> > Feel free to #ifdef around the m_offset member of Address and you can
> have the data race and compile. This seems like a compiler bug to me. If a
> struct/classe by value argument has alignment requirements, then the
> compiler should handle this correctly IMHO. Am I wrong
>
> Or if this isn't a compiler bug, feel free to modify anything that was
> passing Address by value and make it a "const Address &".
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Greg Clayton via lldb-dev

> On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev 
>  wrote:
> 
>> 
>> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
>>  wrote:
>> 
>> I recently updated to Visual Studio 2015 Update 3, which has improved its 
>> diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
>> errors of the following nature:
>> 
>> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
>> 'default_stop_addr': formal parameter with requested alignment of 8 won't be 
>> aligned
>> 
>> The issue comes down to the fact that lldb::Address contains a 
>> std::atomic, and is being passed by value pervasively throughout 
>> the codebase.  There is no way to guarantee that this value is 8 byte 
>> aligned.  This has always been a bug, but until now the compiler just hasn't 
>> been reporting it.
>> 
>> Someone correct me if I'm wrong, but I believe this is a problem on any 
>> 32-bit platform, and MSVC is just the only one erroring.
>> 
>> I'm not really sure what to do about this.  Passing std::atomic's by 
>> value seems wrong to me.
>> 
>> Looking at the code, I don't even know why it needs to be atomic.  It's not 
>> even being used safely.  We'll have a single function write the value and 
>> later read the value, even though it could have been used in the meantime. 
>> Maybe what is really intended is a mutex.  Or maybe it doesn't need to be 
>> atomic in the first place.
>> 
>> Does anyone have a suggestion on what to do about this?  I'm currently 
>> blocked on this as I can't compile LLDB.
> 
> Feel free to #ifdef around the m_offset member of Address and you can have 
> the data race and compile. This seems like a compiler bug to me. If a 
> struct/classe by value argument has alignment requirements, then the compiler 
> should handle this correctly IMHO. Am I wrong

Or if this isn't a compiler bug, feel free to modify anything that was passing 
Address by value and make it a "const Address &".
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Passing std::atomics by value

2016-08-26 Thread Greg Clayton via lldb-dev

> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev 
>  wrote:
> 
> I recently updated to Visual Studio 2015 Update 3, which has improved its 
> diagnostics.  As a result of this, LLDB is uncompilable due to a slew of 
> errors of the following nature:
> 
> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 
> 'default_stop_addr': formal parameter with requested alignment of 8 won't be 
> aligned
> 
> The issue comes down to the fact that lldb::Address contains a 
> std::atomic, and is being passed by value pervasively throughout 
> the codebase.  There is no way to guarantee that this value is 8 byte 
> aligned.  This has always been a bug, but until now the compiler just hasn't 
> been reporting it.
> 
> Someone correct me if I'm wrong, but I believe this is a problem on any 
> 32-bit platform, and MSVC is just the only one erroring.
> 
> I'm not really sure what to do about this.  Passing std::atomic's by 
> value seems wrong to me.
> 
> Looking at the code, I don't even know why it needs to be atomic.  It's not 
> even being used safely.  We'll have a single function write the value and 
> later read the value, even though it could have been used in the meantime.  
> Maybe what is really intended is a mutex.  Or maybe it doesn't need to be 
> atomic in the first place.
> 
> Does anyone have a suggestion on what to do about this?  I'm currently 
> blocked on this as I can't compile LLDB.

Feel free to #ifdef around the m_offset member of Address and you can have the 
data race and compile. This seems like a compiler bug to me. If a struct/classe 
by value argument has alignment requirements, then the compiler should handle 
this correctly IMHO. Am I wrong

> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] Passing std::atomics by value

2016-08-26 Thread Zachary Turner via lldb-dev
I recently updated to Visual Studio 2015 Update 3, which has improved its
diagnostics.  As a result of this, LLDB is uncompilable due to a slew of
errors of the following nature:

D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719:
'default_stop_addr': formal parameter with requested alignment of 8 won't
be aligned

The issue comes down to the fact that lldb::Address contains a
std::atomic, and is being passed by value pervasively throughout
the codebase.  There is no way to guarantee that this value is 8 byte
aligned.  This has always been a bug, but until now the compiler just
hasn't been reporting it.

Someone correct me if I'm wrong, but I believe this is a problem on any
32-bit platform, and MSVC is just the only one erroring.

I'm not really sure what to do about this.  Passing std::atomic's
by value seems wrong to me.

Looking at the code, I don't even know why it needs to be atomic.  It's not
even being used safely.  We'll have a single function write the value and
later read the value, even though it could have been used in the meantime.
Maybe what is really intended is a mutex.  Or maybe it doesn't need to be
atomic in the first place.

Does anyone have a suggestion on what to do about this?  I'm currently
blocked on this as I can't compile LLDB.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

2016-08-26 Thread Ted Woodward via lldb-dev
After the core file is loaded in ProcessElfCore::DoLoadCore, the logic under 
"target create" will merge the ArchSpec of the target and the core, replacing 
the "unknown" OS in the core ArchSpec with "linux" from the target ArchSpec.

Howard, are you loading a target executable, or just the core?

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


-Original Message-
From: Greg Clayton [mailto:gclay...@apple.com] 
Sent: Friday, August 26, 2016 11:08 AM
To: Ted Woodward 
Cc: Howard Hellyer ; Todd Fiala ; 
LLDB 
Subject: Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

It is ok for a core file to not pledge allegiance to an OS, it is ok for the OS 
to be set to "*" or any OS. Linux core files are useless without the main 
executable anyway so these two things should used together to do the right 
thing. When creating the core files you use:

lldb::ProcessSP
ProcessElfCore::CreateInstance (lldb::TargetSP target_sp, lldb::ListenerSP 
listener_sp, const FileSpec *crash_file) {

So you are handed the target. You can get the executable file from the target 
and also check the target's architecture or the main executable's architecture. 
There shouldn't be a problem figuring this out right?

Greg

> On Aug 26, 2016, at 8:17 AM, Ted Woodward via lldb-dev 
>  wrote:
> 
> That works fine for host debug, but not so much for embedded. On Hexagon, we 
> support 2 OS cases – standalone (which means no OS, or an OS that lldb 
> doesn’t know anything about) and Linux. Both our standalone simulator and our 
> Linux generate core dumps using ELFOSABI_NONE. We run lldb on both x86 Linux 
> and Windows, and our core dumps need to work on both.
>  
> Doesn’t lldb get the correct OS from the target when you load them together?
>  
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
> a Linux Foundation Collaborative Project
>  
> From: Howard Hellyer [mailto:hhell...@uk.ibm.com]
> Sent: Friday, August 26, 2016 8:39 AM
> To: Todd Fiala 
> Cc: LLDB ; Ted Woodward 
> 
> Subject: Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value
>  
> Todd Fiala  wrote on 25/08/2016 20:42:31:
> 
> > FWIW, I've taken a few whacks at getting Linux detected better over 
> > the last few years, and haven't yet found a reliable way to detect 
> > it from quite a few samples of cores from a number of different 
> > systems.  We can spend more time looking into it, but that stone has 
> > been turned over several times.
> 
> I spent quite a lot of time looking at the output of readelf too. I was kind 
> of hoping Linux was the only platform not using it's OSABI value, which would 
> have worked. 
> 
> The only other thing I thought of suggesting was having the ELFOSABI_NONE 
> case ifdef'd so that lldb defaults to the platform that it was built for - on 
> the assumption that you are probably opening a core from the machine you are 
> on. (So on Linux ELFOSABI_NONE would mean Linux, on FreeBSD it would mean 
> FreeBSD.) That would have meant lldb behaved differently depending on where 
> it was compiled which seems wrong and would introduce awkward to debug 
> behaviour so I ended up talking myself out of it.
> 
> Howard Hellyer
> IBM Runtime Technologies, IBM Systems
> 
> 
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
> 741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
> 3AU ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Watching reads/writes on optimized variables?

2016-08-26 Thread Greg Clayton via lldb-dev
Maybe the volatile keyword?

volatile int x = 10;

> On Aug 26, 2016, at 9:27 AM, Christian Convey  
> wrote:
> 
> Hi Greg,
> 
> 
>>> 
>>> Does anyone know of a way to minimize or eliminate this problem?
>> 
>> Just take the address of your variable at some point in your code and it 
>> will force it into memory.
> 
> Thanks for your idea.  I can see why taking the variable's address (in
> an expression that's not optimized away) would prompt llvm to allocate
> genuine stack space for it.
> 
> However, I'm still concerned about the following scenario:
> 
>   int foo() {
>  int x;
>  dummy_func(&x); // <-- your idea, AFAICT
>  x = 10;
>  f(x);
>  x = 20;
>  g(x);
>  return x;
>   }
> 
> I suspect it's still possible that the optimizer will (conceptually) replace:
>  x = 10;
>  f(x);
>  x = 20;
>  g(x);
>  return x;
> with:
>  f(10);
>  f(20);
>  return 20;
> 
> It's pretty important to me that at debug-time I detect the fact that
> "X=10" and "X=20" (conceptually) were executed and modified the value
> of "x".  (Whatever approach I use also needs to detect access to "x"
> which occur via aliasing.)
> 
> - Christian

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Watching reads/writes on optimized variables?

2016-08-26 Thread Christian Convey via lldb-dev
Hi Greg,


>>
>> Does anyone know of a way to minimize or eliminate this problem?
>
> Just take the address of your variable at some point in your code and it will 
> force it into memory.

Thanks for your idea.  I can see why taking the variable's address (in
an expression that's not optimized away) would prompt llvm to allocate
genuine stack space for it.

However, I'm still concerned about the following scenario:

   int foo() {
  int x;
  dummy_func(&x); // <-- your idea, AFAICT
  x = 10;
  f(x);
  x = 20;
  g(x);
  return x;
   }

I suspect it's still possible that the optimizer will (conceptually) replace:
  x = 10;
  f(x);
  x = 20;
  g(x);
  return x;
with:
  f(10);
  f(20);
  return 20;

It's pretty important to me that at debug-time I detect the fact that
"X=10" and "X=20" (conceptually) were executed and modified the value
of "x".  (Whatever approach I use also needs to detect access to "x"
which occur via aliasing.)

- Christian
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

2016-08-26 Thread Greg Clayton via lldb-dev
It is ok for a core file to not pledge allegiance to an OS, it is ok for the OS 
to be set to "*" or any OS. Linux core files are useless without the main 
executable anyway so these two things should used together to do the right 
thing. When creating the core files you use:

lldb::ProcessSP
ProcessElfCore::CreateInstance (lldb::TargetSP target_sp, lldb::ListenerSP 
listener_sp, const FileSpec *crash_file)
{

So you are handed the target. You can get the executable file from the target 
and also check the target's architecture or the main executable's architecture. 
There shouldn't be a problem figuring this out right?

Greg

> On Aug 26, 2016, at 8:17 AM, Ted Woodward via lldb-dev 
>  wrote:
> 
> That works fine for host debug, but not so much for embedded. On Hexagon, we 
> support 2 OS cases – standalone (which means no OS, or an OS that lldb 
> doesn’t know anything about) and Linux. Both our standalone simulator and our 
> Linux generate core dumps using ELFOSABI_NONE. We run lldb on both x86 Linux 
> and Windows, and our core dumps need to work on both.
>  
> Doesn’t lldb get the correct OS from the target when you load them together?
>  
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
> Linux Foundation Collaborative Project
>  
> From: Howard Hellyer [mailto:hhell...@uk.ibm.com] 
> Sent: Friday, August 26, 2016 8:39 AM
> To: Todd Fiala 
> Cc: LLDB ; Ted Woodward 
> Subject: Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value
>  
> Todd Fiala  wrote on 25/08/2016 20:42:31:
> 
> > FWIW, I've taken a few whacks at getting Linux detected better over 
> > the last few years, and haven't yet found a reliable way to detect 
> > it from quite a few samples of cores from a number of different 
> > systems.  We can spend more time looking into it, but that stone has
> > been turned over several times. 
> 
> I spent quite a lot of time looking at the output of readelf too. I was kind 
> of hoping Linux was the only platform not using it's OSABI value, which would 
> have worked. 
> 
> The only other thing I thought of suggesting was having the ELFOSABI_NONE 
> case ifdef'd so that lldb defaults to the platform that it was built for - on 
> the assumption that you are probably opening a core from the machine you are 
> on. (So on Linux ELFOSABI_NONE would mean Linux, on FreeBSD it would mean 
> FreeBSD.) That would have meant lldb behaved differently depending on where 
> it was compiled which seems wrong and would introduce awkward to debug 
> behaviour so I ended up talking myself out of it.
> 
> Howard Hellyer
> IBM Runtime Technologies, IBM Systems
> 
> 
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
> 741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Watching reads/writes on optimized variables?

2016-08-26 Thread Greg Clayton via lldb-dev

> On Aug 26, 2016, at 7:25 AM, Christian Convey via lldb-dev 
>  wrote:
> 
> Hi guys,
> 
> I'm trying to use watchpoints to detect user-space reads/writes of an
> arbitrary C/C++ program variable.
> 
> For example:
>   void foo() {
>  int x;   // <-- I'm interested in 'x'
>  x = 10;  // <-- I want to detect this
> 
>  for (int i = 0; i < 4; ++i) {
> x = i;  // <-- And this
> bar(x);
>  }
> 
>  x = 20;  // <-- and this
>  bar(x);
>  baz( &x ); // <-- and any updates to 'x' during this call
>   }
> 
> My concern is that the clang+LLVM will sometimes model "x" using a
> register or constant, rather than with memory.  And so a watchpoint
> might miss some reads/writes to "x".
> 
> Does anyone know of a way to minimize or eliminate this problem?

Just take the address of your variable at some point in your code and it will 
force it into memory.

> 
> Ideas I've considered so far:
> 
> * Compiling the whole program with "-O0".  This might be enough, but
> I'm not sure.
> 
> * Add the "volatile" qualifier to "x".  This might solve the problem,
> but could require countless additions of "volatile" elsewhere as well.
> 
> * Adding a statement of the form "my_dummy_func( &x )".  Assuming this
> reliably causes a memory allocation for "x", this might help.  But I
> wouldn't expect it to reliably preclude "x"'s value from being modeled
> with a register or constant at certain locations in the object code.
> 
> I don't mind modifying compiler/linker flags, but I'd prefer to not
> modify the source code.  I should be able to use most versions of GCC
> and/or Clang/LLVM/LLDB.
> 
> Thanks,
> Christian
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

2016-08-26 Thread Ted Woodward via lldb-dev
That works fine for host debug, but not so much for embedded. On Hexagon, we
support 2 OS cases - standalone (which means no OS, or an OS that lldb
doesn't know anything about) and Linux. Both our standalone simulator and
our Linux generate core dumps using ELFOSABI_NONE. We run lldb on both x86
Linux and Windows, and our core dumps need to work on both.

 

Doesn't lldb get the correct OS from the target when you load them together?

 

--

Qualcomm Innovation Center, Inc.

The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

 

From: Howard Hellyer [mailto:hhell...@uk.ibm.com] 
Sent: Friday, August 26, 2016 8:39 AM
To: Todd Fiala 
Cc: LLDB ; Ted Woodward

Subject: Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

 

Todd Fiala mailto:todd.fi...@gmail.com> > wrote on
25/08/2016 20:42:31:

> FWIW, I've taken a few whacks at getting Linux detected better over 
> the last few years, and haven't yet found a reliable way to detect 
> it from quite a few samples of cores from a number of different 
> systems.  We can spend more time looking into it, but that stone has
> been turned over several times. 

I spent quite a lot of time looking at the output of readelf too. I was kind
of hoping Linux was the only platform not using it's OSABI value, which
would have worked. 

The only other thing I thought of suggesting was having the ELFOSABI_NONE
case ifdef'd so that lldb defaults to the platform that it was built for -
on the assumption that you are probably opening a core from the machine you
are on. (So on Linux ELFOSABI_NONE would mean Linux, on FreeBSD it would
mean FreeBSD.) That would have meant lldb behaved differently depending on
where it was compiled which seems wrong and would introduce awkward to debug
behaviour so I ended up talking myself out of it. 


Howard Hellyer
IBM Runtime Technologies, IBM Systems 







Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] Watching reads/writes on optimized variables?

2016-08-26 Thread Christian Convey via lldb-dev
Hi guys,

I'm trying to use watchpoints to detect user-space reads/writes of an
arbitrary C/C++ program variable.

For example:
   void foo() {
  int x;   // <-- I'm interested in 'x'
  x = 10;  // <-- I want to detect this

  for (int i = 0; i < 4; ++i) {
 x = i;  // <-- And this
 bar(x);
  }

  x = 20;  // <-- and this
  bar(x);
  baz( &x ); // <-- and any updates to 'x' during this call
   }

My concern is that the clang+LLVM will sometimes model "x" using a
register or constant, rather than with memory.  And so a watchpoint
might miss some reads/writes to "x".

Does anyone know of a way to minimize or eliminate this problem?

Ideas I've considered so far:

* Compiling the whole program with "-O0".  This might be enough, but
I'm not sure.

* Add the "volatile" qualifier to "x".  This might solve the problem,
but could require countless additions of "volatile" elsewhere as well.

* Adding a statement of the form "my_dummy_func( &x )".  Assuming this
reliably causes a memory allocation for "x", this might help.  But I
wouldn't expect it to reliably preclude "x"'s value from being modeled
with a register or constant at certain locations in the object code.

I don't mind modifying compiler/linker flags, but I'd prefer to not
modify the source code.  I should be able to use most versions of GCC
and/or Clang/LLVM/LLDB.

Thanks,
Christian
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Linux ELF header e_ident[EI_OSABI] value

2016-08-26 Thread Howard Hellyer via lldb-dev
Todd Fiala  wrote on 25/08/2016 20:42:31:

> FWIW, I've taken a few whacks at getting Linux detected better over 
> the last few years, and haven't yet found a reliable way to detect 
> it from quite a few samples of cores from a number of different 
> systems.  We can spend more time looking into it, but that stone has
> been turned over several times.

I spent quite a lot of time looking at the output of readelf too. I was 
kind of hoping Linux was the only platform not using it's OSABI value, 
which would have worked.

The only other thing I thought of suggesting was having the ELFOSABI_NONE 
case ifdef'd so that lldb defaults to the platform that it was built for - 
on the assumption that you are probably opening a core from the machine 
you are on. (So on Linux ELFOSABI_NONE would mean Linux, on FreeBSD it 
would mean FreeBSD.) That would have meant lldb behaved differently 
depending on where it was compiled which seems wrong and would introduce 
awkward to debug behaviour so I ended up talking myself out of it.


Howard Hellyer
IBM Runtime Technologies, IBM Systems





Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev