[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. It would be good to have a test for the ObjectFileELF part at least. I'm thinking of something similar to what we have in `test/Shell/ObjectFile/PECOFF/symbol.yaml` for COFF... Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2218

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm updated this revision to Diff 302178. aadsm added a comment. Herald added subscribers: MaskRay, emaste. Herald added a reviewer: espindola. Also set the symbol as external when it is weak Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm added a comment. Should I still go ahead with this since @labath implemented the memory allocation on lldb-server? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87868/new/ https://reviews.llvm.org/D87868

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak())) { const uint32_t range_scope = labath wrote: >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak())) { const uint32_t range_scope = clayborg wrote: >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak())) { const uint32_t range_scope = labath wrote: >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak())) { const uint32_t range_scope = aadsm wrote: >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:36 bool lldb_private::InferiorCallMmap(Process *process, addr_t _addr, addr_t addr, addr_t length, unsigned prot, clayborg

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm added inline comments. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak())) { const uint32_t range_scope = labath wrote: >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. In D87868#2309260 , @clayborg wrote: > In D87868#2308515 , @labath wrote: > >> That sounds like a plan. FWIW, here's the implementation I hacked up today: >> >> Status

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. Regardless of the future of the syscall approach, I think limiting ourselves to external mmap symbols is a good idea. And if it fixes your problem -- great. It would be good to have a test for that, though. Maybe something like this would do? static void *mmap() {

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. We probably don't need to check for IsWeak() as we wouldn't want an internal + weak symbol (if there is such a thing) to match and be used. Comment at:

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added a comment. Glad the other mmap was not marked external, that makes this much easier. Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54 +if (sc_list.GetContextAtIndex(i, sc) && +(sc.symbol->IsExternal() || sc.symbol->IsWeak()))

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm updated this revision to Diff 296220. aadsm added a comment. I explored Greg's suggestion of checking if the functions were external symbols. As suspected the overriden mmap is not external, so we can use this check to avoid calling it! It does look to me that Pavel's suggestion is the

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

jingham added a comment. In D87868#2309260 , @clayborg wrote: > In D87868#2308515 , @labath wrote: > >> That sounds like a plan. FWIW, here's the implementation I hacked up today: >> >> Status

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added a comment. In D87868#2308515 , @labath wrote: > That sounds like a plan. FWIW, here's the implementation I hacked up today: > > Status NativeProcessLinux::AllocateMemory(size_t size, uint32_t permissions, >

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. Herald added a subscriber: pengfei. That sounds like a plan. FWIW, here's the implementation I hacked up today: Status NativeProcessLinux::AllocateMemory(size_t size, uint32_t permissions, lldb::addr_t ) {

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added a comment. In D87868#2305345 , @labath wrote: > In D87868#2304680 , @clayborg wrote: > >> That is possible, though how do we figure out the syscall enumeration for >> the mmap syscall reliably for a

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. In D87868#2304680 , @clayborg wrote: > That is possible, though how do we figure out the syscall enumeration for the > mmap syscall reliably for a given system? And if we are remote debugging? > Seems like we are signing up for

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added a comment. In D87868#2303398 , @labath wrote: > A completely different approach would be to avoid the mmap function > completely, and go for the mmap syscall instead. > > That is, instead of setting up registers to fake call to mmap and

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

labath added a comment. A completely different approach would be to avoid the mmap function completely, and go for the mmap syscall instead. That is, instead of setting up registers to fake call to mmap and doing a run-to entry point breakpoint, we could set them up to fake a syscall, and then

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm added a comment. > It seems like calling any 'mmap' definition should work. Is the interposed > mmap implementation failing and correctly returning -1, or is it succeeding > and incorrectly returning -1? In either case, it seems like it's worth > digging into why it's failing /

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

clayborg added a comment. So sounds like first we should verify if the "mmap" for asan is exported or not. If it isn't then this is an easy fix, iterate through all possible matches and avoid any internal versions of mmap and only call externally visible symbols. Antonio, can you verify the

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

jingham added a comment. In D87868#2286313 , @kastiglione wrote: > Should lldb avoid calling asan (or any other sanitizer's) `mmap`? If so, > maybe this function can be sanitizer aware and ignore those symbols. At present, this is a function that gets

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

kastiglione added a comment. Should lldb avoid calling asan (or any other sanitizer's) `mmap`? If so, maybe this function can be sanitizer aware and ignore those symbols. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87868/new/

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

vsk added a comment. It seems like calling any 'mmap' definition should work. Is the interposed mmap implementation failing and correctly returning -1, or is it succeeding and incorrectly returning -1? In either case, it seems like it's worth digging into why it's failing / returning the wrong

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

jingham added a comment. Calling every symbol called mmap you find around till one returns some value makes me a bit nervous. For instance, on macOS dyld has a symbol called "mmap" that isn't meant to be called by anybody but dyld. We probably don't want to accidentally call that. You can

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

aadsm created this revision. aadsm added reviewers: clayborg, jingham, JDevlieghere. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. aadsm requested review of this revision. I found an issue where the expression evaluation doesn't work (for non trivial exprs) when the leak