Implement the ABI for WIndows-x86_64 including register info and calling convention.
Handled nested struct returned in register (SysV doesn't have it supported)
Details
Diff Detail
- Repository
- rLLDB LLDB
Event Timeline
lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp | ||
---|---|---|
1094 | This really isn't correct. There is no reason to assume that everything on x86_64 uses the SysV ABI. Can you convert this to a switch over the OS type and return it from the known OSes (Linux, BSD, and Darwin do conform to SysV, the others can return ABISP(). | |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp | ||
1078 | Ugh, this really feels like copy and paste. Perhaps we should have a .inc file that we can use to replicate the reigster names across the targets. | |
1104 | Nit: the indentation seems off. | |
1203 | Mind using CHAR_BIT instead of 8? | |
1359 | FP80 is not supported on Windows, this should be a hard error. | |
1466 | long double is the same as double on Windows. | |
1492 | lmao, is this a copy paste thing? I think that xmm_reg is better than altivec_reg as ALTIVEC is PPC specific. | |
1573 | Mind using CHAR_BIT instead of 8 please? | |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h | ||
55 | This comment doesn't make sense on Windows - signal handlers don't really apply to Windows. | |
59 | This should be made strict as per the Windows ABI. | |
63 | Can we add an additional test please? In particular, leaf frames are permitted to have unaligned stacks. |
update a new version based on comment.
cleaned some useless comment
change to use CHAR_BIT for readability
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp | ||
---|---|---|
1257–1259 | not relevant, deleted | |
1359 | I changed it to provide a correct error message. Is it better to put an assertion or return here? | |
1466 | got it thanks | |
1811 | thanks | |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h | ||
59 | windows is 16-byte aligned, updated. | |
63 | can you provide more information about this? On windows a leaf function doesn't require a stack frame. But a frame function that doesn't call other function is not required to align the stack |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h | ||
---|---|---|
63 | I'm not sure if the Phabracator comments are pointing to the correct bits of sourcecode - you're talking about CallFrameAddressIsValid right? This is checking the stack frame's Canonical Frame Address -- it is a check that the caller's $rsp was 16-byte aligned before it CALLed a (possibly leaf). It isn't a check of the $rsp value in the callee. The unwinder uses this as a sanity check to detect when the backtrace has bogus values as it walks up the stack. Also, not sure where this comment thread is :) but re. CodeAddressIsValid - return true because this is the x86 ISA where there is no instruction alignment restrictions. This method is for architectures with fixed width instructions (okay, where"fixed width" is greater than 1 :) to help detect backtrace mistakes (again). e.g. aarch64/arm64 instructions are always 4-byte aligned. |
lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp | ||
---|---|---|
1094 | Nit: a switch is clearer and easier to extend. | |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp | ||
1097 | Nit: I think that arch.GetTriple().isOSWindows() is nicer than the explicit check of Win32. | |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h | ||
48 | The comment seems wrong |
lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp | ||
---|---|---|
1097 | got it |
update the CreateDefaultUnwindPlan.
return false or give empty Unwind Plan may cause lldb to crash and break tests on windows.
So for now, copy the SysV-x86_64
Update the GetReturnValueObjectImpl using function CanPassInRegisters to explicitly check
Added support for XMM registers.
Now the step out on a function returns floating point number should have the right value.
To play with it, define a function
float getFloat(float value) { return value; }
set a break point and step out
LLDB will print out the right value if debugging on Windows-x86_64
complete the code for floating point registers and cleaned up
the support for floating point registers fixed TestRegistersIterator update the testcased
the OS check in SysV x64 ABI broke the test SymbolFile/DWARF/debug_loc.s
added a restriction lld to #REQUIRES
Can you elaborate on that? How exactly did it break this test?
added a restriction lld to #REQUIRES
That definitely is not the right solution here. If you add lld to your checkout on windows, then "REQUIRES: lld" will be satisfied, but the test will (I guess) still fail. "XFAIL: system-windows" is probably what you want here, but it would be good to understand why this starts to fail now. Is it because we fail to detect that the file in question there is a linux file, and so the triple comes out like "x86_64--" or something? Maybe we should make the SysV abi plugin match unknown oss too... The only format where we have trouble determining the OS is ELF, and it's a pretty good bet that any elf file will be using the "SysV" ABI...
@labath Hi, you're right about the lld, it didn't make the test pass, instead it became unsupported.
So the problem here is that, llvm-mc constructs the Triple correctly, but lldb does not preserve all the information from it, at least it does not know about the OS type.
Maybe we should make the SysV abi plugin match unknown oss too...
I'm not sure this is the right thing to do, but this should fix the problem.
The only format where we have trouble determining the OS is ELF, and it's a pretty good bet that any elf file will be using the "SysV" ABI...
Can you provide more information about this? It sounds like a bug
It would at least preserve status quo. Not much, but it is something...
The only format where we have trouble determining the OS is ELF, and it's a pretty good bet that any elf file will be using the "SysV" ABI...
Can you provide more information about this? It sounds like a bug
Well.. you could consider it a bug, but it's quite hard to say where that bug is (it's definitely not a bug in lldb or llvm-mc). A sad fact of life is that the ELF files don't usually carry enough information to properly identify the os/environment they were intended to run on. If you run readelf -e on the file generated in the test, you'll get something like
ELF Header: Magic: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00 Class: ELF64 Data: 2's complement, little endian Version: 1 (current) OS/ABI: UNIX - System V ABI Version: 0 Type: REL (Relocatable file) Machine: Advanced Micro Devices X86-64 Version: 0x1 Entry point address: 0x0 Start of program headers: 0 (bytes into file) Start of section headers: 648 (bytes into file) Flags: 0x0 Size of this header: 64 (bytes) Size of program headers: 0 (bytes) Number of program headers: 0 Size of section headers: 64 (bytes) Number of section headers: 9 Section header string table index: 1 Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align [ 0] NULL 0000000000000000 00000000 0000000000000000 0000000000000000 0 0 0 [ 1] .strtab STRTAB 0000000000000000 00000238 000000000000004e 0000000000000000 0 0 1 [ 2] .text PROGBITS 0000000000000000 00000040 0000000000000003 0000000000000000 AX 0 0 4 [ 3] .debug_str PROGBITS 0000000000000000 00000043 000000000000001b 0000000000000001 MS 0 0 1 [ 4] .debug_loc PROGBITS 0000000000000000 0000005e 0000000000000036 0000000000000000 0 0 1 [ 5] .debug_abbrev PROGBITS 0000000000000000 00000094 000000000000002d 0000000000000000 0 0 1 [ 6] .debug_info PROGBITS 0000000000000000 000000c1 000000000000003d 0000000000000000 0 0 1 [ 7] .rela.debug_info RELA 0000000000000000 00000190 00000000000000a8 0000000000000018 8 6 8 [ 8] .symtab SYMTAB 0000000000000000 00000100 0000000000000090 0000000000000018 1 6 8 Key to Flags: W (write), A (alloc), X (execute), M (merge), S (strings), I (info), L (link order), O (extra OS processing required), G (group), T (TLS), C (compressed), x (unknown), o (OS specific), E (exclude), l (large), p (processor specific) There are no program headers in this file.
The elf header has this OS/ABI field which could be used to specify linux, but here it's just set to the generic "none" value (which gets printed as "unix sysv"). There is a value for ELFOSABI_LINUX/ELFOSABI_GNU, which could be used to say "linux", but it seems nobody uses it (I'm not sure what's the background there). I guess that usually wasn't a problem, because tools would just assume they are dealing with binaries for the host platform, but lldb has a strong desire to know what platform is a binary intended for without assuming anything about the environment it's being run it. That desire is understandable, but it kind of collides with reality. Over time, lldb has developed a bunch of heuristics to determine the right OS for a file, but these are just heuristics, and they fail on extremely simple files like the ones used in this test, because they have nothing to go on.
allow SysV_x86_64 ABI to handle unknown OS type
this should fix the test SymbolFile/DWARF/debug_loc.s
lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp | ||
---|---|---|
231 ↗ | (On Diff #206270) | There's NetBSD missing here. This broke our buildbots. I'll fix it but please be more careful in the future. |
I think this change has introduced a dependence on x64.
I tried building with the 32b Visual Studio compiler and there are a number of undefined registers in registercontextwindows_x64.cpp.
This is because the version of _CONTEXT included by winnt.h is for x32 and not x64.
i.e., llvm-project\lldb\source\plugins\process\windows\common\x64\registercontextwindows_x64.cpp(297): error C2039: 'Rax': is not a member of '_CONTEXT'
Thoughts? Maybe this doesn't need to be included when building x32 LLDB?
This seems kind of strange to me. I didn't think you could actually build RegisterContextWindows_x64.cpp on an x86 machine. From the CMake logic, it looks like it's guarded by CMAKE_SYSTEM_PROCESSOR matching either "x86_64" or "AMD64". Do you have any local changes or a specific CMake setup that caused you to run into this?
Thoughts? Maybe this doesn't need to be included when building x32 LLDB?
I would rather be able to build this all on x86 if possible, but I don't think it's the end of the world if there's no nice way to do this. There is plenty of code in LLDB guarded by what platform you're building on/for, this would just be another instance of it.
I can confirm this is reproducible with the master branch of llvm-project cloned from GitHub.
You can reproduce the build failure from an x64 command prompt with this cmake command line:
cmake path-to-llvm-project/llvm -Thost=x64 -DLLVM_ENABLE_PROJECTS="clang;lld;lldb" -DCLANG_DEFAULT_LINKER=lld -DLLVM_ENABLE_ASSERTIONS=ON
And building with:
msbuild ALL_BUILD.vcxproj /m
In this configuration CMAKE_SYSTEM_PROCESSOR is going to report AMD64 and this is going to use the x64 hosted Visual Studio compiler targeting 32 bit Windows. Probably some of these files need to be wrapped in #if defined(_M_X64).
lldb/trunk/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h | ||
---|---|---|
1 ↗ | (On Diff #206270) | Nit; number of dashes '----' is wrong in the headers of the files added |
You might want to have a look at https://reviews.llvm.org/D65409 which might fix (some of?) the issues mentioned here.
I don't know if this is relevant or not, but I thought I'd say that on linux we also effectively aren't able to debug a 64-bit process with a 32-bit lldb(-server). (effectively = that's not entirely true, and the situation is a bit more subtle, but the overall effect is the same).
This really isn't correct. There is no reason to assume that everything on x86_64 uses the SysV ABI. Can you convert this to a switch over the OS type and return it from the known OSes (Linux, BSD, and Darwin do conform to SysV, the others can return ABISP().