[libcxx-commits] [PATCH] D126841: [libunwind] Add more information to eh_frame_hdr version error.

[Fangrui Song via Phabricator via libcxx-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LGTM with some minor issues fixed.

The subject line does not end with a period. Please describe why you need extra information in the diagnostic.



================
Comment at: libunwind/src/EHHeaderParser.hpp:60
   if (version != 1) {
-    _LIBUNWIND_LOG0("Unsupported .eh_frame_hdr version");
+    _LIBUNWIND_LOG("Unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,
+                   version, ehHdrStart);
----------------
The standard diagnostics do not use capitalization: https://llvm.org/docs/CodingStandards.html#error-and-warning-messages libunwind mostly doesn't use capitalization


================
Comment at: libunwind/src/EHHeaderParser.hpp:61
+    _LIBUNWIND_LOG("Unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,
+                   version, ehHdrStart);
     return false;
----------------
Use `static_cast<uint64_t>`. pint_t is 32-bit on ELF32 platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126841