[lldb] Set the correct triple when creating an ArchSpec for windows

[wallace]

Summary

The function SetArchitecture, in the case of windows archs, unsets all the parameters of the triple and thus the arch becomes ELF, which is the default.
This change fixes this issue.

[zturner]

Does this affect the running of the test suite in any way? Is anything fixed or regressed as a result of this?

[zturner]

BTW, you will need to rebase this on top of trunk, and make sure you run clang-format before submitting.

[wallace]

This fixes some issues with arm architectures (where this was unsetting the thumbv7 and was fallbacking to thumb).
This is also needed for https://reviews.llvm.org/D24284, where the objet file requires a correct architecture inferred from the machine arch

[zturner]

In D24283#535405, @wallace wrote:

This fixes some issues with arm architectures (where this was unsetting the thumbv7 and was fallbacking to thumb).
This is also needed for https://reviews.llvm.org/D24284, where the objet file requires a correct architecture inferred from the machine arch

Right, I'm just wondering if when you run ninja check-lldb before and after this patch, do the results differ at all?

[wallace]

oh, thank you, first time i'm sending lldb diffs

[wallace]

rebase

[wallace]

I've run the tests and saw that two tests have been fixed
before:

Test Methods:       1829
Reruns:                0
Success:            1061
Expected Failure:    113
Failure:               0
Error:                 7
Exceptional Exit:      0
Unexpected Success:   11
Skip:                637
Timeout:               0
Expected Timeout:      0

After

Test Methods:       1829
Reruns:                0
Success:            1061
Expected Failure:    115
Failure:               0
Error:                 7
Exceptional Exit:      0
Unexpected Success:    9
Skip:                637
Timeout:               0
Expected Timeout:      0

So Expected Failures increased by two

[zturner]

In D24283#541645, @wallace wrote:

I've run the tests and saw that two tests have been fixed
before:

Test Methods:       1829
Reruns:                0
Success:            1061
Expected Failure:    113
Failure:               0
Error:                 7
Exceptional Exit:      0
Unexpected Success:   11
Skip:                637
Timeout:               0
Expected Timeout:      0

After

Test Methods:       1829
Reruns:                0
Success:            1061
Expected Failure:    115
Failure:               0
Error:                 7
Exceptional Exit:      0
Unexpected Success:    9
Skip:                637
Timeout:               0
Expected Timeout:      0

So Expected Failures increased by two

Actually that means two tests have been broken :) Before they were succeeding even though we thought they should fail, and now they are failing as we would expect them to do. I'm not sure if this is good or bad. :) Is there any way you could run the test suite twice, once with your patch and once without, and see which two tests it is? And maybe post the log files of the two tests now that they are failing?

[wallace]

Hey, apparently this change doesn't affect the test coverage, there are at least two flaky tests that sometimes produce unexpected success: test_step_over_dwo, test_process_interrupt_dwo

I repeatedly ran the tests with and without my change it the flakiness was quite consistent

[zturner]

I can believe that :) Looks good then.

[wallace]

btw, i don't have commit access. Can you commit it or should i request access?

[zturner]

I will commit it either later today or tomorrow.

[wallace]

thanks!

[wallace]

ping

[zturner]

Thanks for the reminder. Will do this today.

lldb/trunk/source/Core/ArchSpec.cpp

if (arch_def_entry) {
             break;
           case llvm::ELF::ELFOSABI_OPENBSD:
             m_triple.setOS(llvm::Triple::OSType::OpenBSD);
             break;
           case llvm::ELF::ELFOSABI_SOLARIS:
             m_triple.setOS(llvm::Triple::OSType::Solaris);
             break;
           }
+        } else if (arch_type == eArchTypeCOFF && os == llvm::Triple::Win32) {
+          m_triple.setVendor(llvm::Triple::PC);
+          m_triple.setOS(llvm::Triple::Win32);
         } else {
           m_triple.setVendor(llvm::Triple::UnknownVendor);
           m_triple.setOS(llvm::Triple::UnknownOS);
         }
         // Fall back onto setting the machine type if the arch by name failed...
         if (m_triple.getArch() == llvm::Triple::UnknownArch)
           m_triple.setArch(core_def->machine);
       }