Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Incorrect usage of DW_OP_constXXX on 32-bit targets #47056

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR48087
Status NEW
Importance P normal
Reported by labath@google.com
Reported on 2020-11-05 02:30:44 -0800
Last modified on 2020-11-06 17:00:24 -0800
Version trunk
Hardware All All
CC dblaikie@gmail.com, jdevlieghere@apple.com, keith.walker@arm.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Dwarf says (section 2.5.1.1. Literal Encodings of DWARF v5):
Operations other than DW_OP_const_type push a value with the generic type, and
if the value of a constant in one of these operations is larger than can be
stored in a
single stack element, the value is truncated to the element size and the low-
order bits are pushed on the stack.

[ In 2.5.1., "generic type" is defined as "an integral type that has the size
of an address on the target machine and unspecified signedness" ]

If we take this code:
void g(long long);
void f() {
  long long x = 0x4247;
  g(x);
  x = 0x474247424742ull;
  g(x);
}
And compile it (with optimizations) with clang for a 32-bit target, we get the
following location list for "x":
                     [0x00000000, 0x0000000f): DW_OP_consts +16967, DW_OP_stack_value
                     [0x0000000f, 0x00000025): DW_OP_consts +78349988939586, DW_OP_stack_value)

This usage is incorrect, because the result of DW_OP_consts should be truncated
to 4 bytes (size of an address). For the first expression, that's mostly fine,
but for the second value, this garbles/truncates the expression value.

gdb will display the truncated (wrong) value for x. lldb will display the
correct value, but only because it's handling of DW_OP_constXX opcodes is non-
conforming (I was about to fix it to make it conforming, before I discovered
this problem). I haven't checked what other consumers do.

gcc deals with this problem by using DW_OP_implicit_value for this variable:
                     [0x00000000, 0x0000001b): DW_OP_implicit_value 0x8 0x47 0x42 0x00 0x00 0x00 0x00 0x00 0x00
                     [0x0000001b, 0x00000031): DW_OP_implicit_value 0x8 0x42 0x47 0x42 0x47 0x42 0x47 0x00 0x00)

I guess llvm should do the same?

We can keep the symmetrical lldb bug for a while for backwards compatibility,
but it would be good to fix that one day....
Quuxplusone commented 3 years ago

Sounds OK to me.

Looks like we should probably generalize some functionality in a few places - the current llvm::DwarfExpression::emitSignedConstant doesn't use any litN encodings, whereas emitConstu does use litN encodings when applicable. Both could be generalized/unified a bit, and have support for implicit_value added when it's out of range for the generic typed value, I think.