Add the ability to dump hex bytes with optional offsets and ASCII bytes.

[clayborg]

Summary

There are sure to be many places where a buffer of bytes need to be dumped in LLVM. This change allows users to quickly dump a buffer of bytes with optional offset prefix and ASCII display:

Example output for raw hex bytes:
55 48 89 e5 48 81 ec 70 04 00 00 48 8d 05 10 02
00 00 4c 8d 05 fd 01 00 00 4c 8b 0d d0 02 00 00

Example output for raw hex bytes with ASCII:
55 48 89 e5 48 81 ec 70 04 00 00 48 8d 05 10 02 UH.?H.?p...H....
00 00 4c 8d 05 fd 01 00 00 4c 8b 0d d0 02 00 00 ..L..?...L..?...

Example output for raw hex bytes with ASCII and offsets/addresses:
0x100000d10: 55 48 89 e5 48 81 ec 70 04 00 00 48 8d 05 10 02 UH.?H.?p...H....
0x100000d20: 00 00 4c 8d 05 fd 01 00 00 4c 8b 0d d0 02 00 00 ..L..?...L..?...

Added unit tests as well.

[clayborg]

The output looks better with a mono spaced font of course.

[zturner]

Just a tip. If you wrap the block in triple backticks it will show up in a fixed-width font in phabricator (see below for an example)

That aside, this looks pretty good, but we have this functionality in a couple of places in LLVM already. It would be nice to centralize. Also, this is just a personal preference, but I kind of prefer the way the existing implementations format this. Maybe you could look at the existing implementations and if you prefer it as well, move the code for them over here?

Here's a sample of what our current output looks like:

bin\llvm-pdbdump raw -stream-data=1 d:\src\llvm\test\debuginfo\pdb\inputs\empty.pdb
...
      0000: 942E3101 E207E554 01000000 0B355641  |..1....T.....5VA|
      0010: 86A0A249 896F9988 FAE52FF0 22000000  |...I.o..../."...|
      0020: 2F4C696E 6B496E66 6F002F6E 616D6573  |/LinkInfo./names|
      0030: 002F7372 632F6865 61646572 626C6F63  |./src/headerbloc|
      0040: 6B000300 00000600 00000100 00001A00  |k...............|
      0050: 00000000 00001100 00000900 00000A00  |................|
      0060: 00000D00 00000000 00000500 00000000  |................|
      0070: 00004191 3201                        |..A.2.|

The code for this is in ScopedPrinter::printBinaryImpl.

Even if we don't combine the two right away, I still prefer this style, so it would be nice if the API matched this style closely enough that it would be possible to have printBinaryBlock delegate to your function in the future with only minor changes.

Some limitations of the existing implementation:

1. Doesn't allow configurable line length. It's always 16-bytes per line.
2. Doesn't allow configurable field-width. Always 4-bytes per field.
3. Doesn't allow prefixing the address with 0x or left-padding with zeros.

Feel free to take on as much or as little of this as you think is useful. But I think we should at least be consistent with the output style.

lib/Support/raw_ostream.cpp

370

This will need to be an uppercase I.

388

Same here.

unittests/Support/NativeFormatTests.cpp

48

I know it's a poorly named file, but these tests should probably go in raw_ostream_test.cpp. NativeFormatTests.cpp was intended specifically to test the functions in NativeFormatting.cpp.

[mehdi_amini]

Cool feature!
I only have minor comments inline.

include/llvm/Support/Format.h

208

This wasn't clear to me until I read the code in raw_ostream that this is the "starting offset" for the offset to print in the margin. I thought it was about indention.

223

I'm not against the "type erasure" with void *, but what about also keeping an overload with an ArrayRef of uint8_t? It could avoid to have to unpack at call sites.

lib/Support/raw_ostream.cpp

373

Early exit:

if (Index >= Size)
  break;

390

This loop could iterate from 0 to i and you wouldn't need this test here.
Repeating the same early exit as the previous loop might be easier to read though.

[zturner]

include/llvm/Support/Format.h

223

I kind of am. Is there a reason why we need to specify void* P, size_t N when we can just specify ArrayRef<uint8_t>?

[clayborg]

Fixed all of the previous inline comments:

• moved unittest to right file
• added support for byte groups so we can display 4 bytes at a time with no spaces by default
• added '|' delimiters so the ASCII looks better when displayed and matches other clients
• fixed early exits from loops
• used ArrayRef<uint8_t> instead of void * and size_t.

[clayborg]

Updates Done status on inlined comments.

[mehdi_amini]

LGTM now. Wait a little to see if @zturner has any other comment.

include/llvm/Support/Format.h

223

I see a reason: it avoids casting at call site when you don't have uint8_t (for instance if you have a ArrayRef<int8_t>).

(However is it dangerous because N here is the number of bytes and if you have an ArrayRef<int> Arr it is too easy to pass Arr.size() for N)

[zturner]

Mostly looks good, just a few nits and I think we're good.

include/llvm/Support/Format.h

208

This should probably be a size_t. Also, maybe it can be an Optional<size_t>, which seems to more closely model what we want to use it for.

230–235

It's a bit confusing to me that there are two almost identical versions of the function with different default values for the parameters. What about having a single version that takes all parameters? If so, I would suggest the signature be something like:

format_hex_bytes(ArrayRef<uint8_t>, bool ShowAsciiBlock = true, uint32_t BytesPerLine=16, uint8_t BytesPerGroup = 4, bool Upper = true, Optional<size_t> FirstByteOffset = 0).

I'm not sure if this is the optimal ordering of parameters, but I tried to arrange this in order from most commonly specified to least commonly specified. Not sure if I got it right though.

If you think it's better to have two versions of the functions, at least make the default values of the arguments be the same.

[mehdi_amini]

include/llvm/Support/Format.h

230–235

Which default argument value is different between the two free functions? I think they are identical.

The default is different on the class ctor though, and it may even be possible to remove the default there as the two free functions are making the uses "nice" enough.

[clayborg]

Took care of requested fixes.

[clayborg]

Let me know if things look good Zach.

include/llvm/Support/Format.h

208

Good idea on the optional. It probably won't use a size_t since that can be 32 bits on some systems and might prevent you from showing a 64 bit address/offset in your output.

231

I think two functions is clearer. I did remove the defaulted args in the ctor. Both default args are the same for format_hex_bytes and format_hex_bytes_with_ascii.

[zturner]

My only other suggestion is to terminate the output when the last byte is printed rather than printing spaces. Since isprint(' ') will be true, you won't be able to distinguish trailing spaces in the input from simply running out of bytes. For example, if you tried to print the string " " then your ascii would just look like | |, and it doesn't provide any value. So I would print that like this instead: | |. Even if the last row doesn't line up, I think that's more clear. For example, the code I linked to earlier prints this:

0000: 942E3101 E207E554 01000000 0B355641  |..1....T.....5VA|
0010: 86A0A249 896F9988 FAE52FF0 22000000  |...I.o..../."...|
0020: 2F4C696E 6B496E66 6F002F6E 616D6573  |/LinkInfo./names|
0030: 002F7372 632F6865 61646572 626C6F63  |./src/headerbloc|
0040: 6B000300 00000600 00000100 00001A00  |k...............|
0050: 00000000 00001100 00000900 00000A00  |................|
0060: 00000D00 00000000 00000500 00000000  |................|
0070: 00004191 3201                        |..A.2.|

And that makes it clear from the ASCII that the byte sequence terminates there instead of having some trailing spaces in it.

Feel free to commit after that.

include/llvm/Support/Format.h

208

Ahh that's a good point.

[clayborg]

I fixed Zach's request to remove the extra spaces in the ASCII and committed with 286316.

include/llvm/Support/Format.h

 //
 //  OS << format("mynumber: %4.5f\n", 1234.412);
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_FORMAT_H
 #define LLVM_SUPPORT_FORMAT_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 #include <cstdio>
 #include <tuple>
 
 namespace llvm {

 ///   OS << format_decimal(0, 5)     => "    0"
 ///   OS << format_decimal(255, 5)   => "  255"
 ///   OS << format_decimal(-1, 3)    => " -1"
 ///   OS << format_decimal(12345, 3) => "12345"
 inline FormattedNumber format_decimal(int64_t N, unsigned Width) {
   return FormattedNumber(0, N, Width, false, false, false);
 }
 
+class FormattedHexBytes {
+  llvm::ArrayRef<uint8_t> Bytes;
+  uint64_t Offset; // If not -1ULL start each line with an appropriate offset.
+  uint32_t NumPerLine; // Number of bytes to show per line.
+  bool Upper;          // Show hex bytes as upper case.
+  bool ASCII;          // Show the ASCII bytes for the hex bytes to the right.
+  friend class raw_ostream;
+
+public:
+  FormattedHexBytes(const void *P, size_t N, uint64_t Offset = -1ULL,
+                    uint32_t NPL = 32, bool U = false, bool A = false)
+      : Bytes(), Offset(Offset), NumPerLine(NPL), Upper(U), ASCII(A) {
+    if (P && N > 0)
+      Bytes = llvm::ArrayRef<uint8_t>(static_cast<const uint8_t *>(P), N);
+  }
+};
+
+inline FormattedHexBytes format_hex_bytes(const void *P, size_t N,
+                                          uint64_t Offset = -1ULL,
+                                          uint32_t NumPerLine = 16) {
+  return FormattedHexBytes(P, N, Offset, NumPerLine);
+}
+
+inline FormattedHexBytes format_hex_bytes_with_ascii(const void *P, size_t N,
+                                                     uint64_t Offset = -1ULL,
+                                                     uint32_t NumPerLine = 16) {
+  return FormattedHexBytes(P, N, Offset, NumPerLine, false, true);
+}
+
 } // end namespace llvm
 
 #endif

include/llvm/Support/raw_ostream.h

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
 #include <system_error>
 
 namespace llvm {
 class format_object_base;
 class FormattedString;
 class FormattedNumber;
+class FormattedHexBytes;
 template <typename T> class SmallVectorImpl;
 
 namespace sys {
 namespace fs {
 enum OpenFlags : unsigned;
 }
 }
 
public:
   raw_ostream &operator<<(const format_object_base &Fmt);
 
   // Formatted output, see the leftJustify() function in Support/Format.h.
   raw_ostream &operator<<(const FormattedString &);
 
   // Formatted output, see the formatHex() function in Support/Format.h.
   raw_ostream &operator<<(const FormattedNumber &);
 
+  // Formatted output, see the formatHex() function in Support/Format.h.
+  raw_ostream &operator<<(const FormattedHexBytes &);
+
   /// indent - Insert 'NumSpaces' spaces.
   raw_ostream &indent(unsigned NumSpaces);
 
   /// Changes the foreground color of text that will be output from this point
   /// forward.
   /// @param Color ANSI color to use, the special SAVEDCOLOR can be used to
   /// change only the bold attribute, and keep colors untouched
   /// @param Bold bold/brighter text, default false

lib/Support/raw_ostream.cpp

if (FN.Hex) {
     llvm::write_integer(Stream, FN.DecValue, IntegerStyle::Integer);
     if (Buffer.size() < FN.Width)
       indent(FN.Width - Buffer.size());
     (*this) << Buffer;
   }
   return *this;
 }
 
+raw_ostream &raw_ostream::operator<<(const FormattedHexBytes &FB) {
+  size_t LineIndex = 0;
+  uint64_t Offset = FB.Offset;
+  const size_t Size = FB.Bytes.size();
+  HexPrintStyle OffsetStyle =
+      FB.Upper ? HexPrintStyle::PrefixUpper : HexPrintStyle::PrefixLower;
+  HexPrintStyle ByteStyle =
+      FB.Upper ? HexPrintStyle::Upper : HexPrintStyle::Lower;
+  while (LineIndex < Size) {
+    if (Offset != -1ULL) {
+      llvm::write_hex(*this, Offset + LineIndex, OffsetStyle,
+                      sizeof(Offset) * 2 + 2);
+      *this << ": ";
+    }
+    // Print the hex bytes for this line
+    uint32_t i = 0;
+    for (i = 0; i < FB.NumPerLine; ++i) {
+      size_t Index = LineIndex + i;
+      if (Index < Size) {
+        if (i)
+          *this << " ";
+        llvm::write_hex(*this, FB.Bytes[Index], ByteStyle, 2);
+      } else {
+        break;
+      }
+    }
+    uint32_t BytesDisplayed = i;
+    if (FB.ASCII) {
+      // Print 3 spaces for any bytes that we didn't print on this line so that
+      // the ASCII bytes always align up correctly and 2 bytes space between
+      // the hex bytes and the ASCII bytes.
+      indent(3 * (FB.NumPerLine - i) + 2);
+      // Print the ASCII char values for each byte on this line
+      for (uint32_t i = 0; i < FB.NumPerLine; ++i) {
+        size_t Index = LineIndex + i;
+        if (Index < Size) {
+          char ch = (char)FB.Bytes[Index];
+          if (isprint(ch))
+            *this << ch;
+          else
+            *this << '.';
+        }
+      }
+    }
+    LineIndex += BytesDisplayed;
+    if (LineIndex < Size)
+      *this << '\n';
+  }
+  return *this;
+}
+
 /// indent - Insert 'NumSpaces' spaces.
 raw_ostream &raw_ostream::indent(unsigned NumSpaces) {
   static const char Spaces[] = "                                "
                                "                                "
                                "                ";
 
   // Usually the indentation is small, handle it with a fastpath.
   if (NumSpaces < array_lengthof(Spaces))

unittests/Support/NativeFormatTests.cpp

 //===- llvm/unittest/Support/NativeFormatTests.cpp - formatting tests -----===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/NativeFormatting.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 #include <type_traits>
 
 using namespace llvm;
 
std::string format_number(double D, FloatStyle Style,
                           Optional<size_t> Precision = None) {
   std::string S;
   llvm::raw_string_ostream Str(S);
   write_double(Str, D, Style, Precision);
   Str.flush();
   return S;
 }
 
+std::string format_hex_bytes(const void *P, size_t N, uint64_t Offset = -1ULL,
+                             uint32_t NumPerLine = 16) {
+  std::string S;
+  llvm::raw_string_ostream Str(S);
+  Str << llvm::format_hex_bytes(P, N, Offset, NumPerLine);
+  Str.flush();
+  return S;
+}
+
+std::string format_hex_bytes_with_ascii(const void *P, size_t N,
+                                        uint64_t Offset = -1ULL,
+                                        uint32_t NumPerLine = 16) {
+  std::string S;
+  llvm::raw_string_ostream Str(S);
+  Str << llvm::format_hex_bytes_with_ascii(P, N, Offset, NumPerLine);
+  Str.flush();
+  return S;
+}
+
 // Test basic number formatting with various styles and default width and
 // precision.
 TEST(NativeFormatTest, BasicIntegerTests) {
   // Simple integers with no decimal.
   EXPECT_EQ("0", format_number(0, IntegerStyle::Integer));
   EXPECT_EQ("2425", format_number(2425, IntegerStyle::Integer));
   EXPECT_EQ("-2425", format_number(-2425, IntegerStyle::Integer));
 
TEST(NativeFormatTest, HexTests) {
   EXPECT_EQ("00000", format_number(0, HexPrintStyle::Lower, 5));
   EXPECT_EQ("000abcde", format_number(0xABCDE, HexPrintStyle::Lower, 8));
 
   // Try printing more digits than can fit in a uint64.
   EXPECT_EQ("0x00000000000000abcde",
             format_number(0xABCDE, HexPrintStyle::PrefixLower, 21));
 }
 
+TEST(NativeFormatTest, HexByteTests) {
+  char b[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l',
+              'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x',
+              'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9'};
+  // Test raw bytes
+  const uint64_t InvalidOffset = -1ULL;
+  // Test invalid input.
+  EXPECT_EQ("", format_hex_bytes(nullptr, 0));
+  EXPECT_EQ("", format_hex_bytes(b, 0));
+  EXPECT_EQ("", format_hex_bytes(nullptr, 16));
+  //----------------------------------------------------------------------
+  // Test simple hex byte output
+  //----------------------------------------------------------------------
+  EXPECT_EQ("61", format_hex_bytes(b, 1));
+  EXPECT_EQ("61 62", format_hex_bytes(b, 2));
+  // Test that 16 bytes get written to a line correctly.
+  EXPECT_EQ("61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70",
+            format_hex_bytes(b, 16));
+  // Test raw bytes with default 16 bytes per line wrapping.
+  EXPECT_EQ("61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70\n71",
+            format_hex_bytes(b, 17));
+  // Test raw bytes with 1 bytes per line wrapping.
+  EXPECT_EQ("61\n62\n63\n64\n65\n66", format_hex_bytes(b, 6, InvalidOffset, 1));
+  // Test raw bytes with 7 bytes per line wrapping.
+  EXPECT_EQ("61 62 63 64 65 66 67\n68 69 6a 6b 6c 6d 6e\n6f 70 71",
+            format_hex_bytes(b, 17, InvalidOffset, 7));
+  // Test raw bytes with 8 bytes per line wrapping.
+  EXPECT_EQ("61 62 63 64 65 66 67 68\n69 6a 6b 6c 6d 6e 6f 70\n71",
+            format_hex_bytes(b, 17, InvalidOffset, 8));
+  //----------------------------------------------------------------------
+  // Test hex bytes with offset
+  //----------------------------------------------------------------------
+  EXPECT_EQ("0x0000000000000000: 61", format_hex_bytes(b, 1, 0x0));
+  EXPECT_EQ("0x0000000000001000: 61", format_hex_bytes(b, 1, 0x1000));
+  EXPECT_EQ("0x0000000000001000: 61\n0x0000000000001001: 62",
+            format_hex_bytes(b, 2, 0x1000, 1));
+  //----------------------------------------------------------------------
+  // Test hex bytes with ASCII.
+  //----------------------------------------------------------------------
+  EXPECT_EQ("61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70  abcdefghijklmnop",
+            format_hex_bytes_with_ascii(b, 16));
+  EXPECT_EQ("61 62 63 64 65 66 67 68  abcdefgh\n"
+            "69 6a 6b 6c 6d 6e 6f 70  ijklmnop",
+            format_hex_bytes_with_ascii(b, 16, InvalidOffset, 8));
+  EXPECT_EQ("61 62 63 64 65 66 67 68  abcdefgh\n"
+            "69 6a 6b 6c              ijkl",
+            format_hex_bytes_with_ascii(b, 12, InvalidOffset, 8));
+  char unprintable[] = {'a', '\x1e', 'b', '\x1f'};
+  // Make sure the ASCII is still lined up correctly when fewer bytes than 16
+  // bytes per line are available. The ASCII should still be aligned as if 16
+  // bytes of hex might be displayed.
+  EXPECT_EQ("61 1e 62 1f                                      a.b.",
+            format_hex_bytes_with_ascii(unprintable, 4));
+  //----------------------------------------------------------------------
+  // Test hex bytes with ASCII with offsets.
+  //----------------------------------------------------------------------
+  EXPECT_EQ("0x0000000000000000: 61 62 63 64 65 66 67 68 "
+            "69 6a 6b 6c 6d 6e 6f 70  abcdefghijklmnop",
+            format_hex_bytes_with_ascii(b, 16, 0));
+  EXPECT_EQ("0x0000000000000000: 61 62 63 64 65 66 67 68  abcdefgh\n"
+            "0x0000000000000008: 69 6a 6b 6c 6d 6e 6f 70  ijklmnop",
+            format_hex_bytes_with_ascii(b, 16, 0, 8));
+  EXPECT_EQ("0x0000000000000000: 61 62 63 64 65 66 67  abcdefg\n"
+            "0x0000000000000007: 68 69 6a 6b 6c        hijkl",
+            format_hex_bytes_with_ascii(b, 12, 0, 7));
+}
+
 TEST(NativeFormatTest, IntegerTests) {
   EXPECT_EQ("-10", format_number(-10, IntegerStyle::Integer));
   EXPECT_EQ("-100", format_number(-100, IntegerStyle::Integer));
   EXPECT_EQ("-1000", format_number(-1000, IntegerStyle::Integer));
   EXPECT_EQ("-1234567890", format_number(-1234567890, IntegerStyle::Integer));
   EXPECT_EQ("10", format_number(10, IntegerStyle::Integer));
   EXPECT_EQ("100", format_number(100, IntegerStyle::Integer));
   EXPECT_EQ("1000", format_number(1000, IntegerStyle::Integer));