This is an archive of the discontinued LLVM Phabricator instance.

[MC] Use the non-EH register mapping in the debug_frame section.
ClosedPublic

Authored by friss on Feb 12 2015, 8:32 AM.

Details

Summary

On 32bits x86 Darwin, the register mappings for the eh_frane and
debug_frame sections are different. Thus the same CFI instructions
should result in different registers in the object file. The
problem isn't target specific though, but it requires that the
mappings for EH register numbers be different from the standard
Dwarf one.

The patch is really ugly. The CFI instructions are emitted with
register operands that are already Dwarf EH register numbers,
allthough they will be used to emit both EH and non-EH frame
descriptions. I first thought the right solution was to encode
LLVM register numbers in the CFI instructions and do the
conversion at object emission time. This however breaks a lot
of tests that where the .cfi_* asm directives are tested (which
means it would break targets not using the integrated assembler).

What this patch does instead is to do a double conversion when
emitting the debug_frame section. Knowing the CFI instruction
references EH reg numbers, it converts them back to LLVM reg num
and again back to the correct Dwarf mapping.

I'll add a test, but I first wanted to reach out for opinions
as this looks like a bandaid rahter than a fix.

Fixes PR22363.

Diff Detail

Event Timeline

friss updated this revision to Diff 19836.Feb 12 2015, 8:32 AM
friss retitled this revision from to [MC] Use the non-EH register mapping in the debug_frame section..
friss added a reviewer: grosbach.
friss updated this object.
friss added a subscriber: Unknown Object (MLST).

Adding Rafael to the review as it seems he touched EmitCFIInstruction last/most

rafael edited edge metadata.Feb 12 2015, 12:18 PM

It looks reasonable to me.

You could store the LLVM register numbers, but you would also need to convert in the asm printer and parser as the cfi instructions can unfortunately accept a numeric value.

friss added a comment.Feb 12 2015, 1:10 PM

OK, I'll respin with a testcase.

The patch storing the LLVM register numbers is big, touches every target and would disrupt out of tree targets. Quite a big hammer for a very small gain so I'm glad you find this reasonable.

grosbach edited edge metadata.Feb 12 2015, 1:22 PM

Oof. The patch itself seems reasonable. I tend to agree with your thought that it may not be a root cause. Do you have any ideas about what a deeper solution would look like?

-Jim

friss updated this revision to Diff 20764.Feb 26 2015, 8:06 AM
friss edited edge metadata.

Now that I taugth llvm-dwarddump to dump debug_frame, add a test.
Code patch unchanged.

rafael accepted this revision.Feb 26 2015, 9:35 AM
rafael edited edge metadata.

LGTM

This revision is now accepted and ready to land.Feb 26 2015, 9:35 AM
This revision was automatically updated to reflect the committed changes.