[Lldb-commits] [lldb] [lldb] fix the "RegisterValue::SetValueFromData" method for 128-bit integer registers (PR #163646)

[Matej Košík via lldb-commits]

sedymrak wrote:

> Thanks for the reproducer. Please include it in the PR description as well, as it may be the closest thing we get to a test case.

I am not sure if I understand correctly what you meant I should do.
I have edited my first post on this matter to include the instructions how one can reproduce the problem.
Is this what you meant?

> 
> I'm still not clear how you found this, but if the answer is:
> 
>     * You spotted it by reading code
> 
>     * You have a downstream target with registers like this
> 
>     * You are working on some NDA'd thing

The problem floated to the surface when we adapted LLDB for debugging programs written in a [custom programming language](https://codasip.com/products/codasip-studio/codal/) that supports non-standard integer sizes.

> 
> Then the conclusion is the same. Hard to test this with an upstream target.

I am not aware of any platform where this bug could be directly reproduced.

I am not sure how much it would help, but I plan (tomorrow) to look at [related unit-tests](https://github.com/llvm/llvm-project/blob/77cb19d7aac29215a8cb2f58e35e790b0c94d799/lldb/unittests/Utility/RegisterValueTest.cpp). I need to know:
- Why they did not scream when I changed the `RegisterValue::SetValueFromData` method?
- Can I add a test that will cover the faulty branch?

At worst I will learn why it cannot be done...

> 
> We have faked targets in testing before, so I think we could fake an AArch64 where V registers were not vectors, as this type is set in the XML sent from the debug server. `lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py` is an example of this.
> 
> Then again, I found that there's no public API for this call. Running an expression on a fake target like that could work but it'd be a lot of mock stuff to implement (for a start, we'd need to pretend it can JIT code).
> 
> We have some tests that put a simulated debug sever in front of a real one, but that's too much effort for this fix.
> 
> Let me think about it for a bit. If I don't come up with anything then I'll approve this, as it does look correct to me.

+1


https://github.com/llvm/llvm-project/pull/163646