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

2019-12-21 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 2 inline comments as done. jankratochvil added inline comments. Comment at: lldb/source/Expression/IRMemoryMap.cpp:577-586 if (lldb_private::Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS)) { LLDB_LOGF(log,

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

2019-12-21 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 234935. jankratochvil marked an inline comment as done. Herald added a subscriber: emaste. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71498/new/ https://reviews.llvm.org/D71498 Files:

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

2019-12-21 Thread 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 static FPtrTy CastToFPtr(void *VPtr) { - return reinterpret_cast(reinterpret_cast(VPtr)); + return reinterpret_cast(VPtr); }

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

2019-12-21 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. jankratochvil marked an inline comment as done. Closed by commit rGdf6879ec0227: [lldb] Fix ARM32 inferior calls (authored by jankratochvil). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2019-12-21 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 2 inline comments as done. jankratochvil added inline comments. Comment at: lldb/source/Host/common/HostInfoBase.cpp:250 + reinterpret_cast( + HostInfoBase::ComputeSharedLibraryDirectory))); labath wrote: > jankratochvil

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

2019-12-21 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D71498#1785458 , @labath wrote: > In other places you're replacing a reinterpret_cast with two c casts. `reinterpret_cast` and a `(c cast)` have the same behavior and as `c cast` is shorter I did prefer it. But I see

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

2019-12-21 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thanks for tracking these down! Comment at: lldb/source/Expression/IRMemoryMap.cpp:577-586 if (lldb_private::Log *log =

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

2019-12-18 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment. In D71498#1786341 , @clayborg wrote: > As I am reading this, I just wanted to send out a note of something else that > can cause crashes in ARM/Thumb code. For anyone working with ARM/Thumb on > systems that don't use the ARM

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

2019-12-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D71498#1786343 , @jankratochvil wrote: > In D71498#1786319 , @clayborg wrote: > > > For the printf style statement, we can't use just one cast to "uintptr_t" > > because on 32 bit

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

2019-12-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. As I am reading this, I just wanted to send out a note of something else that can cause crashes in ARM/Thumb code. For anyone working with ARM/Thumb on systems that don't use the ARM and Thumb BKPT instruction when setting software breakpoints (like all lldb linux and

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

2019-12-16 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D71498#1786319 , @clayborg wrote: > For the printf style statement, we can't use just one cast to "uintptr_t" > because on 32 bit systems it won't be converted to 64 bit. That pointer-to-`uint64_t` is now used for

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

2019-12-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. > In other places you're replacing a reinterpret_cast with two c casts. > I guess this was done because two c++ casts were too long (?). In these cases > the second cast is not really needed, as uintptr_t->addr_t should convert > automatically. I think I'd prefer that

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

2019-12-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Seems like it might be nice to have a macro defined in a LLDB header file like lldb-defines.h. Something like: #define PTR_TO_U64(x) ((uint64_t)(uintptr_t)(x)) Then we can make a unit test to verify it works on all systems. Seem like this issue will pop up all over

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

2019-12-16 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment. Symbols sizes are being zeroed out Correct without D63540 : https://pastebin.ubuntu.com/p/fKGxFfwyxV/ Incorrect with D63540 : https://pastebin.ubuntu.com/p/XBTScYYkhq/ Repository: rG LLVM Github

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

2019-12-16 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. Could you check symtabs what symbols are located at: th1/fr0 with pc value of 0x102f0, symbol name is '_start' vs. th1/fr0 with pc value of 0xfe52, no symbol/function name is known. ? Or maybe just attached the main executable as `_start=0x102f0` is there I

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

2019-12-16 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment. This issue is being caused by wrong address being written to memory somewhere while single stepping though i have reached the exact problem but the logs seem to suggest it. LLDB Log without D63540

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

2019-12-16 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D71498#1785486 , @omjavaid wrote: > error: Can't run the expression locally: Interpreter doesn't handle one of > the expression's opcodes OK, I see it is a different error. I will try to reproduce also that one.

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

2019-12-16 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment. I still seem to get the same issue after applying this patch and D63540 . echo -e '#include \nint main(void){\nsync();return 0;}'|./bin/clang -g -x c -;./bin/lldb -o 'file ./a.out' -o 'b main' -o r -o 'p (void)sync()' (lldb) file

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

2019-12-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Yeah, this looks like it was fun to track down. I'm going to comment on both patches here, since they are really similar... Overall, I'm not very enthusiastic about the addr_t wrapper -- I'm not sure it would be even possible because that's a part of the public API. It

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

2019-12-13 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil created this revision. jankratochvil added reviewers: omjavaid, labath. jankratochvil added a project: LLDB. Herald added subscribers: kristof.beyls, aprantl. jankratochvil added a reviewer: jasonmolenda. echo -e '#include \nint main(void){\nsync();return 0;}'|./bin/clang -g -x c