[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D63165#1611924 <https://reviews.llvm.org/D63165#1611924>, @asmith wrote:

> I can make the one simplification but Im not sure what you are asking as far as the refactoring. Provide an example and I will try to accommodate your request. I'm not going to be able to spend much more time on lldb patches though.


I think you should change the `CreateRegisterInfoInterface` in `NativeRegisterContextWindows_x86_64.cpp` to something like:

  static RegisterInfoInterface *
  CreateRegisterInfoInterface(const ArchSpec &target_arch) {
    assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) &&
           "Register setting path assumes this is a 64-bit host");
    switch (target.arch.GetAddressByteSize()) {
    case 8:
      return new RegisterContextWindows_x86_64(target_arch);
    case 8:
      return new RegisterContextWindows_i386(target_arch);
    default:
      llvm_unreachable("Unsupported byte size!");
  }

Then you should be able to remove all of the `g_private_reg_interface` business from `RegisterContextWindows_x86_64.cpp`, as the RegisterContextWindows_x86_64 will only ever be called with a 64-bit ArchSpec. In fact, since you already have NativeRegisterContextWindows_WoW64, I am not sure if even the `4` branch is needed, as in case of GetAddressByteSize() == 4, we should end up in `CreateRegisterInfoInterface` in `NativeRegisterContextWindows_WoW64` (which already correctly constructs a `RegisterContextWindows_i386`).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63165/new/

https://reviews.llvm.org/D63165