DavidSpickett added inline comments.
================
Comment at:
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:221
+
+ uint8_t *src = const_cast<uint8_t *>(data_sp->GetBytes());
+ if (src == nullptr) {
----------------
seehearfeel wrote:
> DavidSpickett wrote:
> > Possibly not needed const cast.
> If no const cast, build failed:
>
> ```
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:221:12:
> error: cannot initialize a variable of type 'uint8_t *' (aka 'unsigned char
> *') with an rvalue of type 'const uint8_t *' (aka 'const unsigned char *')
> uint8_t *src = data_sp->GetBytes();
> ^ ~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> ```
Yes, for the way you've written it that makes sense. But I realise I was
looking at the wrong thing here. It's fine that the GetBytes() is const because
this is a WriteRegister call, that's expected.
You should be fine using `const uint8_t*` here, because you can still increment
that pointer and memcpy from it. Since the pointer itself can change but the
data it points to is const.
https://godbolt.org/z/9M9oqr575 might explain it better.
================
Comment at:
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:60
+ virtual bool WriteGPR() = 0;
+ virtual bool WriteFPR() = 0;
+};
----------------
seehearfeel wrote:
> DavidSpickett wrote:
> > This doesn't look right, I'd expect `bool ... () override;` here.
> Maybe we should leave it as is, the other archs do the same thing,
> they will be override in the following file which is not implemented now:
> lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_loongarch64.h/cpp,
> otherwise build failed:
>
> ```
> In file included from
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.cpp:19:
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:57:18:
> error: only virtual member functions can be marked 'override'
> bool ReadGPR() override;
> ^~~~~~~~
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:58:18:
> error: only virtual member functions can be marked 'override'
> bool ReadFPR() override;
> ^~~~~~~~
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:59:19:
> error: only virtual member functions can be marked 'override'
> bool WriteGPR() override;
> ^~~~~~~~
> /home/loongson/llvm.git/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_loongarch64.h:60:19:
> error: only virtual member functions can be marked 'override'
> bool WriteFPR() override;
> ^~~~~~~~
> 4 errors generated.
> ```
Yes I think I was looking at one of the core file versions and didn't realise.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138407/new/
https://reviews.llvm.org/D138407
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
