[Input] Fix segfault when sysroot file is missing during reproduce tarball replay#1305
Conversation
Deepak Shirke (deepakshirkem)
changed the title
Parth (parth-07)
left a comment
Parth (parth-07)
left a comment
There was a problem hiding this comment.
I don't understand why is the PR modifying an already exiting test and how is it fixing anything by not modifying the source-code at all.
Deepak Shirke (deepakshirkem)
force-pushed
the
fix/sysroot-script-crash-818
branch
from
9445040 to
3dbdd39
Compare
|
Hi Parth (@parth-07), The PR has been update. It now includes the fix in Input.cpp and a correct test. Please review when you get a chance! |
Deepak Shirke (deepakshirkem)
force-pushed
the
fix/sysroot-script-crash-818
branch
from
3dbdd39 to
e9edbcf
Compare
|
Parth (@parth-07) ping. |
Deepak Shirke (deepakshirkem)
force-pushed
the
fix/sysroot-script-crash-818
branch
from
e9edbcf to
f8cb02c
Compare
| RUN: %tar %gnutaropts -xvf %t.tar -C %t.reproduce --strip-components=1 | ||
| RUN: rm -rf %t.reproduce/SharedLibrary | ||
| RUN: rm -rf %t.dir/lib64/lib1.so | ||
| RUN: cd %t.reproduce && %not %link --no-threads @%t.response 2>&1 | %filecheck %s |
There was a problem hiding this comment.
Why do we need --no-threads here?
There was a problem hiding this comment.
Why do we need --no-threads here?
was used to match the original link command in the reproduce tarball's response.txt will update the test to use %link with %linkopts instead,
| @@ -0,0 +1,26 @@ | |||
| #UNSUPPORTED: windows, reproduce_fail | |||
There was a problem hiding this comment.
Why is this test unsupported for windows and reproduce_fail?
There was a problem hiding this comment.
After checking existing reproduce tests (Reproduce.test, ArchiveFile.test, Namespec.test), all use the same #UNSUPPORTED: windows, reproduce_fail pattern since our test uses --reproduce flag and Unix-specific commands, so this is correct.
| RUN: %tar %gnutaropts -xvf %t.tar -C %t.reproduce --strip-components=1 | ||
| RUN: rm -rf %t.reproduce/SharedLibrary | ||
| RUN: rm -rf %t.dir/lib64/lib1.so | ||
| RUN: cd %t.reproduce && %not %link --no-threads @%t.response 2>&1 | %filecheck %s |
There was a problem hiding this comment.
The fix for #818 is not avoiding the crash and reporting an error message. It should instead be making reproducer work correctly for the case when the shared library should be found under the sysroot.
There was a problem hiding this comment.
Yeah this makes sense to me. Thank-You
| if (!InputMem) | ||
| InputMem = createMemoryArea(FileName, PConfig.getDiagEngine()); | ||
| if (!InputMem) | ||
| return false; // File does not exist; |
There was a problem hiding this comment.
This fix is correct for the case when the reproduce tarball is tampered, but it is a wrong fix for #818. In the issue, there is no tampering of the reproduce tarball. The core issue is not that the linker is crashing, but that the linker is incorrectly handling the sysroot during reproduce tarball.
Can you create a separate issue for the link crashing when the files has been removed from the reproduce tarball/setup (or when the mapping.ini file contains incorrect mapping) and then link this PR with that issue?
…e does not exist When replaying a reproduce tarball, ELD crashes with a segfault if a linker script references a file that is not present in the sysroot. The crash occurs in Input::resolvePathMappingFile() where createMemoryArea() returns nullptr when the file does not exist, but the function continues to call setMemArea(nullptr) and returns true, incorrectly signaling success to the caller. This allows a null MemArea Input to reach readAndProcessInput(), which eventually dereferences the null pointer inside ScriptLexer causing a segfault. Fixes qualcomm#1335 Signed-off-by: deepakshirkem <deepakshirke509@gmail.com>
Deepak Shirke (deepakshirkem)
force-pushed
the
fix/sysroot-script-crash-818
branch
from
f8cb02c to
042826d
Compare
|
Parth (@parth-07) Please review one more time. |
2 participants
ELD crashes with a segfault when replaying a reproduce tarball where a file referenced by a linker script is missing from the tarball or the mapping.ini contains an incorrect mapping pointing to a non-existent file.
The root cause is that
Input::resolvePathMappingFile()callscreateMemoryArea(), which returnsnullptrwhen the file does not exist. However, the function still callssetMemArea(nullptr)and returnstrue, allowing a nullMemAreato propagate downstream. This eventually reachesreadAndProcessInput(), where it is dereferencedby
ScriptLexer, resulting in a crash.A new test
MissingMappingFileInputhas been added to reproduce and verify the fix.Fixes #1335
Related to #818
cc quic-areg Parth (@parth-07) Shankar Easwaran (@quic-seaswara)