compnerd added inline comments.
================
Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1094
+ if (arch_type == llvm::Triple::x86_64 &&
+ os_type != llvm::Triple::OSType::Win32) {
return ABISP(new ABISysV_x86_64(process_sp));
----------------
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()`.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1078
+ return g_register_infos;
+}
+
----------------
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.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1104
+ addr_t func_addr, addr_t return_addr,
+ llvm::ArrayRef<addr_t> args) const {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
----------------
Nit: the indentation seems off.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1203
+ }
+ uint32_t byte_size = (bit_width + (8 - 1)) / 8;
+ Status error;
----------------
Mind using `CHAR_BIT` instead of `8`?
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1359
+ } else {
+ // FIXME - don't know how to do 80 bit long doubles yet.
+ error.SetErrorString(
----------------
FP80 is not supported on Windows, this should be a hard error.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1466
+ } else if (*byte_size == sizeof(long double)) {
+ // Don't handle long double since that can be encoded as 80 bit
+ // floats...
----------------
`long double` is the same as `double` on Windows.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1492
+ if (byte_size && *byte_size > 0) {
+ const RegisterInfo *altivec_reg =
+ reg_ctx->GetRegisterInfoByName("xmm0", 0);
----------------
lmao, is this a copy paste thing? I think that `xmm_reg` is better than
`altivec_reg` as ALTIVEC is PPC specific.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp:1573
+ uint32_t field_byte_width = field_bit_width / 8;
+ uint32_t field_byte_offset = field_bit_offset / 8 + data_byte_offset;
+
----------------
Mind using `CHAR_BIT` instead of `8` please?
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h:55
+ // in other environments there can be a large number of different functions
+ // involved in async traps.
+ bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
----------------
This comment doesn't make sense on Windows - signal handlers don't really apply
to Windows.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h:59
+
+ if (cfa & (8ull - 1ull))
+ return false; // Not 8 byte aligned
----------------
This should be made strict as per the Windows ABI.
================
Comment at: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h:63
+ return false; // Zero is not a valid stack address
+ return true;
+ }
----------------
Can we add an additional test please? In particular, leaf frames are permitted
to have unaligned stacks.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62213/new/
https://reviews.llvm.org/D62213
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
