[PATCH] D150079: [BPF][DebugInfo] Show CO-RE relocations in llvm-objdump

Mon Sep 4 23:50:52 PDT 2023

jhenderson added a comment.

Only a couple of nits remaining from me.

================
Comment at: llvm/unittests/DebugInfo/BTF/BTFParserTest.cpp:56
     BTF::Header Header = {};
-    // no types
+    // no types.
     struct S {
----------------
Now I'm being really nit-picky - here and in the `E` struct below it should be capitalized "No" not "no".

================
Comment at: llvm/unittests/DebugInfo/BTF/BTFParserTest.cpp:565
+  D.addType({D.addString("18"), mkInfo(BTF::BTF_KIND_DECL_TAG), {0}});
+  D.addTail((uint32_t)-1);
+  D.addType({D.addString("19"), mkInfo(BTF::BTF_KIND_TYPE_TAG), {0}});
----------------
eddyz87 wrote:
> jhenderson wrote:
> > `std::numeric_limits::max<uint32_t>()` might be clearer?
> [[ https://www.kernel.org/doc/html/latest/bpf/btf.html#btf-kind-decl-tag | Documentation ]] refers to value `-1` (for unsigned field :) ), so I think it's better to keep `-1` here to avoid confusion.
Fair enough.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150079