Shorter generated function names, unify address rendering

- Skip leading zeroes `sub_00401245` -> `sub_401245`, like IDA Pro and
  Binary Ninja
- Generate all function names and addresses from a single place
- Drive-by: Fix comparison in `HumanReadableDuration()`

PiperOrigin-RevId: 482440847
Change-Id: I684aef88abac6d2a04b808b26d4927af4cb1cf23

Author: cblichmann

basic_block.cc

@@ -18,6 +18,9 @@
 #include <ostream>
 
 #include "third_party/zynamics/binexport/call_graph.h"
+#include "third_party/zynamics/binexport/util/format.h"
+
+using security::binexport::FormatAddress;
 
 BasicBlock::Cache BasicBlock::cache_;
 
@@ -57,8 +60,7 @@ BasicBlock* BasicBlock::Create(BasicBlockInstructions* instructions) {
 void BasicBlock::Render(std::ostream* stream, const CallGraph& call_graph,
                         const FlowGraph& flow_graph) const {
   for (const auto& instruction : *this) {
-    *stream << std::hex << std::setfill('0') << std::uppercase << std::setw(8)
-            << instruction.GetAddress() << " ";
+    *stream << FormatAddress(instruction.GetAddress()) << " ";
     instruction.Render(stream, flow_graph);
     std::pair<Comments::const_iterator, Comments::const_iterator> comments =
         call_graph.GetComments(instruction.GetAddress());

call_graph.cc

@@ -20,8 +20,11 @@
 #include <ostream>
 
 #include "third_party/zynamics/binexport/flow_graph.h"
+#include "third_party/zynamics/binexport/util/format.h"
 #include "third_party/zynamics/binexport/util/hash.h"
 
+using security::binexport::FormatAddress;
+
 namespace {
 
 bool EdgeFunctionIsNull(const EdgeInfo& edge) { return edge.function_ == 0; }
@@ -59,8 +62,8 @@ void CallGraph::Render(std::ostream* stream,
                        const FlowGraph& flow_graph) const {
   for (const auto& function_address : functions_) {
     const Function* function = flow_graph.GetFunction(function_address);
-    *stream << std::hex << std::setfill('0') << std::uppercase << std::setw(8)
-            << function_address << " " << std::setfill(' ') << std::setw(8)
+    *stream << FormatAddress(function_address) << " " << std::setfill(' ')
+            << std::setw(8)
             << (function->GetType(false) != Function::TYPE_STANDARD
                     ? Function::GetTypeName(function->GetType(false))
                     : "")
@@ -81,11 +84,9 @@ void CallGraph::Render(std::ostream* stream,
 
   for (const auto& edge : edges_) {
     const Function* function = flow_graph.GetFunction(edge.target_);
-    *stream << std::hex << std::setfill('0') << std::uppercase << std::setw(8)
-            << edge.function_->GetEntryPoint() << ":" << std::hex
-            << std::setfill('0') << std::uppercase << std::setw(8)
-            << edge.source_ << " -> " << std::hex << std::setfill('0')
-            << std::uppercase << std::setw(8) << edge.target_ << "    "
+    *stream << FormatAddress(edge.function_->GetEntryPoint()) << ":"
+            << FormatAddress(edge.source_) << " -> "
+            << FormatAddress(edge.target_) << "    "
             << edge.function_->GetName(Function::DEMANGLED) << " -> "
             << (function ? function->GetName(Function::DEMANGLED) : "") << "\n";
   }

dump_writer.cc

@@ -19,6 +19,7 @@
 
 #include "third_party/zynamics/binexport/call_graph.h"
 #include "third_party/zynamics/binexport/flow_graph.h"
+#include "third_party/zynamics/binexport/util/format.h"
 
 namespace security::binexport {
 

function.cc

@@ -24,9 +24,12 @@
 
 #include "third_party/absl/log/check.h"
 #include "third_party/absl/log/log.h"
-#include "third_party/absl/strings/ascii.h"
 #include "third_party/absl/strings/str_cat.h"
 #include "third_party/zynamics/binexport/call_graph.h"
+#include "third_party/zynamics/binexport/util/format.h"
+
+using security::binexport::FormatAddress;
+using security::binexport::FormatFunctionName;
 
 int Function::instance_count_ = 0;
 Function::StringCache Function::string_cache_;
@@ -48,19 +51,16 @@ Function::~Function() {
 
 void Function::Render(std::ostream* stream, const CallGraph& call_graph,
                       const FlowGraph& flow_graph) const {
-  *stream << std::hex << std::setfill('0') << std::uppercase << std::setw(8)
-          << GetEntryPoint() << "    " << GetModuleName()
+  *stream << FormatAddress(GetEntryPoint()) << "    " << GetModuleName()
           << (GetModuleName().empty() ? "" : ".") << GetName(DEMANGLED) << "\n";
   for (const auto& basic_block_ptr : basic_blocks_) {
     basic_block_ptr->Render(stream, call_graph, flow_graph);
     *stream << "\n";
   }
 
   for (const auto& edge : edges_) {
-    *stream << std::hex << std::setfill('0') << std::uppercase << std::setw(8)
-            << edge.source << " -> " << std::hex << std::setfill('0')
-            << std::uppercase << std::setw(8) << edge.target << " "
-            << edge.GetTypeName() << "\n";
+    *stream << FormatAddress(edge.source) << " -> "
+            << FormatAddress(edge.target) << " " << edge.GetTypeName() << "\n";
   }
   if (!edges_.empty()) {
     *stream << "\n";
@@ -140,8 +140,7 @@ std::string Function::GetName(Name type) const {
   if (HasRealName()) {
     return type == MANGLED || demangled_name_.empty() ? name_ : demangled_name_;
   }
-  return absl::StrCat("sub_", absl::AsciiStrToUpper(absl::StrCat(absl::Hex(
-                                  GetEntryPoint(), absl::kZeroPad8))));
+  return FormatFunctionName(GetEntryPoint());
 }
 
 bool Function::HasRealName() const { return !name_.empty(); }
@@ -198,9 +197,8 @@ int Function::GetBasicBlockIndexForAddress(Address address) const {
     }
   }
 
-  LOG(WARNING) << absl::StrCat("No basic block for ",
-                               absl::Hex(address, absl::kZeroPad8), " in ",
-                               GetEntryPoint());
+  LOG(WARNING) << absl::StrCat("No basic block for ", FormatAddress(address),
+                               " in ", GetEntryPoint());
   return basic_blocks_.size();
 }
 

reader/call_graph.cc

@@ -22,6 +22,7 @@
 #include "third_party/absl/memory/memory.h"
 #include "third_party/absl/strings/str_cat.h"
 #include "third_party/zynamics/binexport/reader/graph_utility.h"
+#include "third_party/zynamics/binexport/util/format.h"
 
 namespace security::binexport {
 
@@ -57,7 +58,7 @@ void VertexPropertyFromVertexProto(const BinExport2::CallGraph::Vertex& vertex,
     vertex_property->flags |= CallGraph::kVertexName;
   }
   if (!(vertex_property->flags & CallGraph::kVertexName)) {
-    vertex_property->name = absl::StrCat("sub_", absl::Hex(vertex.address()));
+    vertex_property->name = FormatFunctionName(vertex.address());
   }
   if (vertex.has_module_index()) {
     vertex_property->module_name = proto.module(vertex.module_index()).name();

util/format.cc

@@ -17,6 +17,10 @@ std::string FormatAddress(Address address) {
   return absl::StrFormat("%016X", address);
 }
 
+std::string FormatFunctionName(Address address) {
+  return absl::StrFormat("sub_%X", address);
+}
+
 std::string HumanReadableDuration(double seconds) {
   std::string result;
 
@@ -38,9 +42,9 @@ std::string HumanReadableDuration(double seconds) {
     absl::StrAppend(&result, (need_space ? " " : ""), full_minutes, "m");
     need_space = true;
   }
-  if (full_seconds > 0 || absl::ToInt64Milliseconds(full) > 0) {
+  if (full_seconds > 0 || full > absl::ZeroDuration()) {
     absl::StrAppend(&result, (need_space ? " " : ""), full_seconds);
-    if (absl::ToInt64Milliseconds(full) > 0) {
+    if (full > absl::ZeroDuration()) {
       absl::StrAppend(&result, ".", absl::ToInt64Milliseconds(full) / 10);
     }
     absl::StrAppend(&result, "s");

util/format.h

@@ -22,7 +22,7 @@
 
 namespace security::binexport {
 
-// Returns a hexadecimal string representation of an address suitable for log
+// Returns a hexadecimal string representation of an address, suitable for log
 // lines and display in UIs. The returned string will always be either 8 or 16
 // uppercase hexadecimal characters, depending on whether the address is larger
 // than 0xFFFFFFFF. In any case, the string is left padded with zeroes.
@@ -34,6 +34,12 @@ namespace security::binexport {
 //   FormatAddress(0x7FF00000004926F4) => "7FF00000004926F4"
 std::string FormatAddress(Address address);
 
+// Returns an artificial name for the function at address, suitable for display
+// in UIs. The returned string will start with "sub_" and uppercase hexadecmial
+// characters. The address is _not_ formatted with leading zeroes, following
+// what IDA Pro and Binary Ninja do.
+std::string FormatFunctionName(Address address);
+
 // Formats a duration given in seconds or as an absl::Duration into a human
 // readable time based on hours. Maximum precision is 1/100th of a second.
 //

util/format_test.cc

@@ -29,6 +29,14 @@ TEST(FormatUtilTest, FormatAddress) {
   EXPECT_THAT(FormatAddress(0x7FF00000004926F4), StrEq("7FF00000004926F4"));
 }
 
+TEST(FormatUtilTest, FormatFunctionName) {
+  EXPECT_THAT(FormatFunctionName(0x08), StrEq("sub_8"));
+  EXPECT_THAT(FormatFunctionName(0x59DE50), StrEq("sub_59DE50"));
+  EXPECT_THAT(FormatFunctionName(0x00000001004940B0), StrEq("sub_1004940B0"));
+  EXPECT_THAT(FormatFunctionName(0x7FF00000004926F4),
+              StrEq("sub_7FF00000004926F4"));
+}
+
 TEST(FormatUtilTest, HumanReadableDuration) {
   EXPECT_THAT(HumanReadableDuration(3600), StrEq("1h"));
   EXPECT_THAT(HumanReadableDuration(1800), StrEq("30m"));