[PATCH] D91152: [obj2yaml] - Dump section offsets in some cases.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 12 00:54:06 PST 2020


jhenderson added a comment.

I mostly want to make sure we don't result in invalid output caused by missing the corner case. If all that happens is that Offset is produced unnecessarily, it's probably fine either way, but what I don't want is Offset to be omitted due to an incorrect calculation, when it shouldn't be, as this will lead to an invalid output, which might mask a problem a user is trying to locate. As things stand, I think we can run into that situation.



================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:480-483
+    if (Sec.Type != ELF::SHT_NOBITS)
+      ExpectedOffset = SecHdr.sh_offset + SecHdr.sh_size;
+    else
+      ExpectedOffset = SecHdr.sh_offset;
----------------
By the way, I think it feels more natural to do the special case first (i.e. `Sec.Type == ELF::SHT_NOBITS`), before the default case, as it is easier to add further cases in the future, but it's not a big deal either way.


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

https://reviews.llvm.org/D91152



More information about the llvm-commits mailing list