[PATCH] D42937: [DWARF] Make llvm-dwp handle DWARF v5 string offsets tables and indexed strings.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: string_offsets_mixed.s:40-46
+# With DWARF v5 we need to match up CUs with their corresponding string offsets table
+# contributions (in order to properly process the contribution headers) and hence need
+# to remap and write them unit-by-unit to the output file. The pre-v5 implementation
+# did not need to worry about this because it could just rewrite the input file's string
+# offsets table in one chunk. The test therefore ensures that llvm-dwp rewrites the
+# string offsets table contributions in the order they appear in the input file section,
+# and not in the order in which their corresponding CUs appear in the index table.
----------------
wolfgangp wrote:
> dblaikie wrote:
> > Oh... I hadn't realized/understood/thought about this.
> > 
> > That's kind of awkward - mixing blobs with headers and blobs without headers in the same section (str_offsets) & then having to use the CU/TUs to disambiguate/dictate how to parse those chunks.
> > 
> > Can we avoid that? Could we just say v4 and v5 are incompatible? Have a flag or something that checks.
> > 
> > Then we could always walk the str_offsets alone, either expecting header'd sections (which I hope are self descriptive - once you know you're reading a v5 str_offsets, you don't need to consult the CU for anything to do that?) or non-header'd sections (where you just treat every word as a string offset without consideration for how those are divided into contributions)
> > 
> > 
> > (Ah, I see you mentioned/highlighted that in the patch description too)
> > That's kind of awkward - mixing blobs with headers and blobs without headers in the same section (str_offsets) & then having to use the CU/TUs to disambiguate/dictate how to parse those chunks.
> > 
> > Can we avoid that? Could we just say v4 and v5 are incompatible? Have a flag or something that checks.
> > 
> That would have been the simpler solution, but llvm-dwarfdump is already handling the same thing, so I thought it would be inconsistent if llvm-dwp didn't handle it too. We also would have to reject any dwp input files with mixed units that were created by non-llvm tools.
> 
> It would certainly be easy to reject any mixing of v5 and v4 units. 
> Pity, though, since it's already working...
I'd be happy to hear some other people's opinions on this (Adrian & Paul?).

It seems pretty unfortunate to have this split between versions making a bunch of complexity to support like this.


https://reviews.llvm.org/D42937