[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:61
+  if (!COFFObj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info)
+    return UUID::fromOptionalData(pdb_info->PDB70.Signature);
+
----------------
Is there a specific reason you used this particular encoding of the UUID, or did you do that just because it was the easiest? I am asking because I have a reason to have this use a somewhat different encoding. :)

Let me elaborate: I think there are two things we can want from the UUID here: The first one is for it to match the UUID encoding we get from other sources (so that they can agree on whether they are talking about the same binary). The second one is for the uuid encoding to match the "native" UUID format of the given platform.

Right now, this implementation achieves neither. :) It fails the second criterion because the UUID strings comes out in different endianness than what the windows native tools produce (I'm using `dumpbin` as reference here.). And it also fails the first one, because e.g. minidump reading code parses the UUID differently <D60501>.

Now, for windows, these two criteria are slightly at odds with one another. In order to fully match the dumpbin format, we would need to have some kind of a special field for the "age" bit. But lldb has no such concept, and there doesn't seem to be much need to introduce it. However, including the "age" field in the "uuid" seems like the right thing to do, as two files with different "ages" should be considered different for debug info matching purposes (at least, that's what my limited understanding of pdbs tells me. if some of this is wrong, please let me know). So, in <D60501> I made a somewhat arbitrary decision to attach the age field to the UUID in big endian.
That's the format that made most sense to me, though that can certainly be changed (the most important thing is for these things to stay in sync).

So, if you have a reason to use a different encoding, please let me know, so we can agree on a consistent implementation. Otherwise, could you change this to use the same UUID format as the minidump parser?


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

https://reviews.llvm.org/D56229