Fix the remainder of warnings for the Windows build.

[zturner]

Summary

Makes the Windows build 99% free of warnings. In particular, this change addresses the following classes of warnings:

• CMake Windows build incorrectly passed -Wl,--start-group and -Wl,--end-group to the command line
• Bit-shift too large or negative causes undefined behavior (e.g. (size_t)x >> 32 on x86)
• GetVersionEx API declared deprecated in Windows 8.1 SDK

[tfiala]

I'll look at this today.

[tfiala]

Hey Zachary,

I have a few comments on the data type changes above.

Let me know if you can address that and still get your warnings fixed.

Thanks!

include/lldb/Core/DataBufferMemoryMap.h

111

I think you may need to do something more like:

#include <limits>
lldb::offset_t length = std::numeric_limits<lldb::offset_t>::max().

MacOSX/iOS have cases where 32-bit code debugs 64-bit code, and in cases like that using the host data types does the wrong thing. I'd keep it lldb::offset_t and just fix the warning by what I suspect is the real issue, which is using SIZE_MAX as the initializer.

140

Likewise, keep this one lldb::offset_t.

source/Core/DataBufferMemoryMap.cpp

129 ↗

(On Diff #9894)

This disappears with change to keep lldb::offset_t

[zturner]

include/lldb/Core/DataBufferMemoryMap.h

111

Hmm, am I misunderstanding the purpose of this function? In the end, this results in a call to CreateFileMapping() in this process, and mmap() on linux. So the mapping is being created inside this process, for which size_t accurately dictates how much memory can be mapped. 32-bit code can debug 64-bit code, but you still can't map more than 4GB into the 32-bit process, which is what could happen if offset_t is the argument type.

It's a little confusing though, so let me know if I've misunderstood.

[tfiala]

I think the challenge is that once we start veering away from the common datatypes that are guaranteed to maintain the right maximum size of data for a target that is possibly not the host, we can end up inadvertently starting to change data types in code that is used on multiple CPUs in between the target and the debugger client. So I think the general philosophy has been to use the generic lldb:: and lldb_private:: types all the way to the final function call, and just do the static_cast<> to the appropriate type at the system/OS call layer.

In this particular case it might not be a big deal, but it then puts pressure on whatever calls this interface to decide maybe it can be a size_t. And to resolve that, another piece of code that might sit in multiple contexts, some of which may convey data across a 64/32/64 boundary, could lose the upper 32 bits. So it can have a domino effect of causing more types in more generic code to solve a warning/data-type size difference by using what appears right - the host OS native type - which could be wrong for the system as a whole.

So it's more a philosophy of safety that I'm appealing to in not changing that to a size_t.

[zturner]

Added comment explaining the use of size_t instead of offset_t.

[tfiala]

Thanks, Zachary. That seems good.

I'll go ahead and check that in.

[tfiala]

Submitted:

svn commit
Sending CMakeLists.txt
Sending include/lldb/Core/DataBufferMemoryMap.h
Sending source/Core/ConnectionSharedMemory.cpp
Sending source/Core/DataBufferMemoryMap.cpp
Sending source/Host/windows/Host.cpp
Sending source/Plugins/Platform/Windows/PlatformWindows.cpp
Transmitting file data ......
Committed revision 210035.

include/lldb/Core/DataBufferMemoryMap.h

     /// @param[in] length
     ///     The size in bytes that should be mapped starting \a offset
     ///     bytes into the file. If \a length is \c SIZE_MAX, map
-    ///     as many bytes as possible.
+    ///     as many bytes as possible.  Even though it may be possible
+    ///     for a 32-bit host debugger to debug a 64-bit target, size_t
+    ///     still dictates the maximum possible size that can be mapped
+    ///     into this process.  For this kind of cross-arch debugging
+    ///     scenario, mappings and views should be managed at a higher
+    ///     level.
     ///
     /// @return
     ///     The number of bytes mapped starting from the \a offset.