[PATCH] D89071: [SystemZ/z/OS] Add GOFFObjectFile class and details of GOFF file format

[Yusra Syeda via Phabricator via llvm-commits]

yusra.syeda marked an inline comment as not done.
yusra.syeda added inline comments.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:165
+  // name and then grab only that many characters but for now this works fine
+  uint16_t Continuations = 0;
+  while (true) {
----------------
kpn wrote:
> jhenderson wrote:
> > What limits this to being specifically `uint16_t` in size?
> I believe the maximum record length even with continuations is 32KB. I don't know if saving two bytes of stack is worth making people reading the code doubletake, though.
> 
> A check to make sure this 32KB limit is not exceeded is needed.
Thanks, I will add that check.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:338
+  }
+  llvm_unreachable("unable to get symbol section");
+  return section_iterator(SectionRef(Sec, this));
----------------
jhenderson wrote:
> Is this really unreachable? What happens if there is no symbol section in the file?
Yes, this should be unreachable and is added as an extra safety check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89071