[DebugInfo] Fix LineTest byteswap for cross-targeting builds

[daltenty]

Summary

The byte swap fix for big endian hosts in 9782c922cb21 (for D81570)
swaps based on the host endianess, but for cross-targeting builds (i.e.
big endian host targeting little endian) the host-endianess won't
necessarily match the generated DWARF. This change updates the test
to use symmetrical constants so the results aren't endian dependent.

[ikudrin]

The patch looks appropriate, but now I think that maybe it would have been enough to simply use symmetrical constants for these particular tests?

[jhenderson]

In D82827#2121798, @ikudrin wrote:

The patch looks appropriate, but now I think that maybe it would have been enough to simply use symmetrical constants for these particular tests?

Testing endianness behaviour of the code under test is not what the original change was about, so getting rid of this byte-swapping code entirely would make the testing simpler, I agree. @daltenty, are you happy to do that? It should be a fairly straightforward change, I believe.

[daltenty]

In D82827#2122013, @jhenderson wrote:

In D82827#2121798, @ikudrin wrote:

The patch looks appropriate, but now I think that maybe it would have been enough to simply use symmetrical constants for these particular tests?

Testing endianness behaviour of the code under test is not what the original change was about, so getting rid of this byte-swapping code entirely would make the testing simpler, I agree. @daltenty, are you happy to do that? It should be a fairly straightforward change, I believe.

That makes sense to me, I will update to remove the byteswap and add a set of symmetrical constants for the big-endian case.

[daltenty]

• Use an Optional big-endian output string rather than trying to byte swap

[daltenty]

In D82827#2122786, @daltenty wrote:

In D82827#2122013, @jhenderson wrote:

In D82827#2121798, @ikudrin wrote:

The patch looks appropriate, but now I think that maybe it would have been enough to simply use symmetrical constants for these particular tests?

Testing endianness behaviour of the code under test is not what the original change was about, so getting rid of this byte-swapping code entirely would make the testing simpler, I agree. @daltenty, are you happy to do that? It should be a fairly straightforward change, I believe.

That makes sense to me, I will update to remove the byteswap and add a set of symmetrical constants for the big-endian case.

Ended up adding an optional big endian version of the message instead, as I think it comes out a bit cleaner than constructing an optional version of the input using the symmetrical constant, but let me know if you have a preference for that version instead.

[jhenderson]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

1475

It's possible you may have misunderstood what we were suggesting. Rather than adding another argument, I think @ikudrin and I were thinking of changing the 0x1234567890abcdef to 0x1234567878563412, and so on in the other tests, so that the error message would be the same regardless of endianness. That way the test is completely agnostic of endianness and there's no need to query the host/target.

1479–1480

This would be the same as above.

1524–1525

This would be 0x12343412.

[ikudrin]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

1475

Right, that was the idea.

[daltenty]

• Use symmetrical constants instead

[daltenty]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

1475

Yes, I definitely did. This is a much better solution, updated as suggested.

[ikudrin]

LGTM. Thanks!

[jhenderson]

Thanks for fixing this!

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

 
 struct TruncatedExtendedOpcodeFixture
     : public TestWithParam<std::tuple<uint64_t, uint64_t, uint8_t,
                                       ValueAndLengths, StringRef, StringRef>>,
       public TruncatedOpcodeFixtureBase {
   void SetUp() {
     std::tie(BodyLength, OpcodeLength, Opcode, Operands, ExpectedOutput,
              ExpectedErr) = GetParam();
-    // Swap the byte order of the operands on big endian hosts, so that the raw
-    // bytes are always in the same order. ValLen.Value is a uint64_t, so make
-    // sure to shift the value back to the actually used bits for the
-    // appropriate type.
-    if (sys::IsBigEndianHost)
-      for (LineTable::ValueAndLength &ValLen : Operands)
-        if (ValLen.Length != LineTable::SLEB &&
-            ValLen.Length != LineTable::ULEB &&
-            ValLen.Length != LineTable::Byte) {
-          sys::swapByteOrder(ValLen.Value);
-          if (ValLen.Length == LineTable::Long)
-              ValLen.Value >>= 32;
-          if (ValLen.Length == LineTable::Half)
-              ValLen.Value >>= 48;
-        }
   }
 
   uint64_t OpcodeLength;
 };
 
 TEST_P(TruncatedExtendedOpcodeFixture, ErrorForTruncatedExtendedOpcode) {
   if (!setupGenerator())
     return;
Values(
                         "malformed uleb128, extends past end"),
         // Truncated opcode:
         std::make_tuple(
             2, 9, /*ArbitraryOpcode=*/0x7f, ValueAndLengths(), "",
             "unexpected end of data at offset 0x31 while reading [0x31, 0x32)"),
         // Truncated operands:
         std::make_tuple(
             3, 9, DW_LNE_set_address,
             ValueAndLengths{{0x1234567890abcdef, LineTable::Quad}},
             "DW_LNE_set_address",
             "unexpected end of data at offset 0x32 while reading [0x32, 0x3a)"),
         std::make_tuple(
             10, 9, DW_LNE_set_address,
-            ValueAndLengths{{0x1234567890abcdef, LineTable::Quad}},
-            "DW_LNE_set_address (<parsing error> ef cd ab 90 78 56 34)",
+            ValueAndLengths{{0x1234567878563412, LineTable::Quad}},
+            "DW_LNE_set_address (<parsing error> 12 34 56 78 78 56 34)",
             "unexpected end of data at offset 0x39 while reading [0x32, 0x3a)"),
         std::make_tuple(3, 6, DW_LNE_define_file,
                         ValueAndLengths{{'a', LineTable::Byte},
                                         {'\0', LineTable::Byte},
                                         {1, LineTable::ULEB},
                                         {1, LineTable::ULEB},
                                         {1, LineTable::ULEB}},
                         "DW_LNE_define_file",
Values(
                         "unable to decode LEB128 at offset 0x00000036: "
                         "malformed uleb128, extends past end"),
         std::make_tuple(3, 2, DW_LNE_set_discriminator,
                         ValueAndLengths{{1, LineTable::ULEB}},
                         "DW_LNE_set_discriminator",
                         "unable to decode LEB128 at offset 0x00000032: "
                         "malformed uleb128, extends past end"),
         std::make_tuple(
             6, 5, /*Unknown=*/0x7f,
-            ValueAndLengths{{0x12345678, LineTable::Long}},
-            "Unrecognized extended op 0x7f length 5 (<parsing error> 78 56 34)",
+            ValueAndLengths{{0x12343412, LineTable::Long}},
+            "Unrecognized extended op 0x7f length 5 (<parsing error> 12 34 34)",
             "unexpected end of data at offset 0x35 while reading [0x32, "
             "0x36)")), );
 
 TEST_P(TruncatedStandardOpcodeFixture, ErrorForTruncatedStandardOpcode) {
   if (!setupGenerator())
     return;
   LineTable &LT = setupTable();
   LT.addStandardOpcode(Opcode, Operands);