[PATCH] D40842: [ObjectYAML] Update obj2yaml and yaml2obj for .debug$H section

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/ObjectYAML/CodeViewYAMLTypeHashing.cpp:51-54
+  BinaryStreamReader Reader(DebugT, llvm::support::little);
+  DebugHSection DHS;
+  cantFail(Reader.readInteger(DHS.Version));
+  cantFail(Reader.readInteger(DHS.HashAlgorithm));
----------------
ruiu wrote:
> zturner wrote:
> > majnemer wrote:
> > > Seeing as how nothing stops MSFT from using the debug$H section for something of their own invention, maybe we should have a magic four byte sequence at the section start to identify that we made it?
> > Once we have this all working, I plan to send a modified copy of the PE/COFF spec to MS and see about getting it officially in the spec.  That said, wouldn't your concern already be addressed by the 16-bit `Version` field?
> If Microsoft adds the same section with a different meaning, and if they decide to add a version field at the beginning of the section (it's not an unreasonable assumption), they would conflict, no?
If they add the same section with different meaning then it doesn't matter what we add to this section, because it could still conflict.  If we're going to use the same section name at all, we have to at least agree on how to parse the header.

So if they add the same section name with different meaning, and we can't agree on a format that doesn't lead to a conflict, we would probably just have to change the section name entirely.  I'm not really that opposed to adding another 4-byte magic field to the beginning, but it seems unnecessary since we already have 4 bytes at the beginning which we can treat as "magic".


https://reviews.llvm.org/D40842