Handle applications with user-defined mmap wrappers.#1195
Conversation
gc00
left a comment
gc00
left a comment
There was a problem hiding this comment.
In the "Conversation" tab for this PR, could you write a little bit about the motivation? I'm guessing that when an end user adds a wrapper for 'mmap', there can be an infinite recursion. Is that the issue?
I've also noted inline what may be a bug concerning converting .. to ..
I assume that the 'fixup! Dl-wrapper memory handling fixes.' commit will be rebased into the prior commit before pushing this in.
Anyway, I really like this commit. I can see where it also fixes other corner cases, that were needed to support 'mmap'.
I'm approving in advance.
jalib/jfilesystem.cpp
Outdated
| len--; | ||
| } | ||
|
|
||
| size_t lastSlash = 0; |
There was a problem hiding this comment.
I suggest: int lastSlash = -1; It means that you will also change to:
for (int i = 0; i < len; i++) {
below. The constant 0 could be confused by the reader with index 0, even though you test forfname == '/'above.
src/plugin/dl/dlwrappers.cpp
Outdated
| if (rpath != NULL) { | ||
| *rpathStr = strtab + rpath->d_un.d_val; | ||
| *rpathStr = dmtcp::Util::replace(*rpathStr, "$ORIGIN", dirname); | ||
| Util::replace(rpathStr, &strtab[rpath->d_un.d_val], "$ORIGIN", dirname); |
There was a problem hiding this comment.
'man ld.so' says that ${ORIGIN} is also valid. We should also test for that.
Also, we're not supporting $LIB, ${LIB}, $PLATFORM, or ${PLATFORM.
We should at least provide a FIXME comment here.
Ooptionally, we could add code to test for this and issue a warning/error.
|
And there also seems to be some issue with the build. I'm not sure what it is, but it would be nice to fix this before pushing it in. Locally, this works for WSL Ubuntu 22.04. If you're not able to figure out why the build is failing, let's talk together. I'm trying to instrument autotest.py better. |
karya0
force-pushed
the
dev/karya0/init_mmap
branch
from
c71c64f to
2d7a0c5
Compare
|
Thanks for the comments, @gc00. I am still working on this PR in a rush, so will update the conversation tab once I am done with testing. In a nutshell, the issue is with a user who has custom mmap wrappers in a selinux environment on arm64 that breaks with dmtcp.
|
It is not available on older GCC releases.
Inlining causes dlsym addr lookup to use wrong return address, resulting in getting symbols from the wrong library.
karya0
force-pushed
the
dev/karya0/init_mmap
branch
from
59955c3 to
49b4eb5
Compare
karya0
force-pushed
the
dev/karya0/init_mmap
branch
from
49b4eb5 to
0854ef2
Compare
2 participants
This PR addresses several wrapper initialization issues exposed on 64-bit ARM machines as observed by a user. Overall, the initialization logic is made leaner with code removal related to static initialization, etc. In addition, a couple of new tests, mmap1 and selinux1 are introduced that will help catch future regressions. The commits are all self-contained and don't have dependencies. However, they are bundled together here because they are all addressing the same general theme and all were required to unblock the user.