Page MenuHomePhabricator

Fix PR33875 by distinguishing between DWO and clang modules

Authored by aprantl on Jul 21 2017, 3:33 PM.



The DWO handling code can get confused by clang modules which also use skeleton CUs to point to the object file with the full debug info. This patch detects whether an object is a "real" DWO or a clang module and prevents LLDB from interpreting clang modules as DWO. This fixes the regression in TestWithModuleDebugging.

Diff Detail


Event Timeline

aprantl created this revision.Jul 21 2017, 3:33 PM

(Presumably this could use a test case?)

aprantl updated this revision to Diff 107734.Jul 21 2017, 3:40 PM

No with testcase of sorts.

jingham accepted this revision.Jul 21 2017, 4:00 PM

I was under the impression that this was found by a test case failure.

Anyway, given that we've got different kinds of DWARF CU's, wouldn't it be better to add an attribute describing them, rather than using semi-unrelated pieces of information like this? I wouldn't hold up this patch on that suggestion, but this does seem kind of indirect...

This revision is now accepted and ready to land.Jul 21 2017, 4:00 PM
clayborg accepted this revision.Jul 21 2017, 4:05 PM

Looks like there already is a test case that was failing as Jim mentioned. Accepting based on that.

This revision was automatically updated to reflect the committed changes.
labath edited edge metadata.Jul 28 2017, 11:19 AM

I looked at this briefly last week but I could not find a good fix in the amount of time I had left. This fixes the current test failure, but it does not fix the underlying bug, which is that we (sometimes?) set the compile unit offset for non-dwo, non-dsym DIEs as 0. This works fine if all compile units are regular dwarf as we have logic to locate the containing CU if the offset is zero. However, it can fail in case of mixed dwarf and dwo compile units. My guess is you could still reproduce this bug with a so file that has a dwo compile unit at offset zero.

That said, I don't think this makes things any worse, so I have no objections to this patch.