Page MenuHomePhabricator

[DebugInfo] Use DW_OP_LLVM_convert_generic after sign/zero exts
AcceptedPublic

Authored by dstenb on Mar 13 2020, 10:48 AM.

Details

Summary

This is a follow-up to D76145.

DW_OP_LLVM_convert is used to do sign- and zero extensions in
DIExpressions. When using DWARFv5, that LLVM operation is translated
into DWARFv5's DW_OP_convert operation.

DWARFv5 section 2.5.1.4 specifies the following for arithmetical and
logical operations:

"Operands of an operation with two operands must have the same type,
either the same base type or the generic type."

So when we have used DW_OP_LLVM_convert in an expression, we need to
make sure that any arithmetical and logical operations operating on that
element have operands of the same type. As we currently don't have a
good interface for detecting what type is on top of the stack, and we
add arithmetical and logical operations, e.g. appending expressions like
{DW_OP_constu 1, DW_OP_plus}, in many places, this patch attempts to
alleviate such cases by introduced a new operation,
DW_OP_LLVM_convert_generic, which converts the top of the stack back to
the generic type.

This operation is appended to the sign/zero extension expression that
DIExpression::getExtOps() produces.

Diff Detail

Event Timeline

dstenb created this revision.Mar 13 2020, 10:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2020, 10:48 AM

Mechanically LGTM to me. I'm not sure if this shouldn't be done in DwarfExpression instead.

Mechanically LGTM to me. I'm not sure if this shouldn't be done in DwarfExpression instead.

So that we basically insert a DW_OP_convert 0x0 after each chain of DW_OP_convert operations in the input DIExpression? That sounds reasonable, and it should have a lot smaller impact on the code. I wonder though if it would be limiting to only have the DwarfExpression world type ware if/when we later want on to support more base type-based operations (e.g. DW_OP_const_type), but perhaps not?

vsk accepted this revision.Mar 16 2020, 4:37 PM

It seems sensible to limit conversions back to the generic type as much as possible, so this approach lgtm.

This revision is now accepted and ready to land.Mar 16 2020, 4:37 PM
mib added a subscriber: mib.Dec 7 2020, 3:27 PM

Hi @dstenb, any update on this ? I tried to look for DW_OP_LLVM_convert_generic in the codebase but couldn't find any occurence ... Thanks.

In D76146#2438388, @mib wrote:

Hi @dstenb, any update on this ? I tried to look for DW_OP_LLVM_convert_generic in the codebase but couldn't find any occurence ... Thanks.

Hi! Sorry for the radio silence here!

I did not land this since this would break sign-extended cases like this:

intX_t local = (intX_t)signed_val_with_smaller_width;

where intX_t is an integer that is wider than the generic type in DWARF. That is, we would incorrectly truncate the value for such cases.

I guess one solution to that would be to be more conservative in adding DW_OP_LLVM_convert_generic in the DIExpression expressions, and avoid adding that when no operation will be applied on the sign-extended value, but we don't really know if more operations will be added later on when adding the convert expression to DIExpression, so I guess we instead would want to insert those generic conversions when appending operations to the DIExpression. Another solution could be to postpone the emission of the generic convert operations to DwarfExpression, and only do that there when required, as @aprantl suggests.