[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 6 inline comments as done.
jankratochvil added inline comments.


================
Comment at: lldb/source/Core/PluginManager.cpp:89
 template <typename FPtrTy> static FPtrTy CastToFPtr(void *VPtr) {
-  return reinterpret_cast<FPtrTy>(reinterpret_cast<intptr_t>(VPtr));
+  return reinterpret_cast<FPtrTy>(VPtr);
 }
----------------
The source is a `void *` and destination is `bool (*PluginInitCallback)()` so there is no intermediate integer cast needed: [[ https://stackoverflow.com/a/2613393/2995591 | Casting pointer to object to void * in C++ ]]
It was caught by D71707 as `intptr_t` is dangerous.



================
Comment at: lldb/source/Expression/IRExecutionUnit.cpp:351-352
+    m_jitted_functions.push_back(
+        JittedFunction(function.getName().str().c_str(), external,
+                       (lldb::addr_t)(uintptr_t)fun_ptr));
   }
----------------
clayborg wrote:
> This can probably just be:
> 
> ```
>  JittedFunction(function.getName().str().c_str(), external, (uintptr_t)fun_ptr));
> ```
> since this is a function call and "fun_ptr" will be correctly converted to a lldb::addr_t
Used `reinterpret_cast` instead of a C cast:
```
warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting]
```



================
Comment at: lldb/source/Host/common/HostInfoBase.cpp:250
+      reinterpret_cast<void *>(
+          HostInfoBase::ComputeSharedLibraryDirectory)));
 
----------------
The source is a `static bool ComputeSharedLibraryDirectory(FileSpec &file_spec);` and destination is `void *` so there is no intermediate integer cast needed: [[ https://stackoverflow.com/a/2613393/2995591 | Casting pointer to object to void * in C++ ]]
It was caught by D71707 as `intptr_t` is dangerous.



================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:358
       ConstantInt::get(llvm::Type::getInt64Ty(m_module->getContext()),
-                       reinterpret_cast<uint64_t>(result_decl), false);
+                       reinterpret_cast<uintptr_t>(result_decl), false);
 
----------------
The source is a `clang::NamedDecl *result_decl` and destination is `uint64_t V`.



================
Comment at: lldb/source/Plugins/Process/POSIX/CrashReason.cpp:141
+                 reinterpret_cast<uintptr_t>(info.si_upper),
+                 reinterpret_cast<uintptr_t>(info.si_addr));
     return str;
----------------
Destination is always `lldb::addr_t`.



================
Comment at: lldb/source/Plugins/Process/POSIX/CrashReason.cpp:147
   return GetCrashReasonString(reason,
-                              reinterpret_cast<lldb::addr_t>(info.si_addr));
+                              reinterpret_cast<uintptr_t>(info.si_addr));
 }
----------------
Destination is `lldb::addr_t`.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71498/new/

https://reviews.llvm.org/D71498