Fix crashes when loading a core dump
ClosedPublic

Authored by christiangagneraud on Sep 25 2017, 3:11 AM.

Details

Summary

The code assumed that the ELF contains some sections, but core dumps don't
have any of them, leading to nullptr dereference and out of bound indexing.

PS: Is it the only way to do pull request? I have to upload raw diff?

Test Plan

When i run the testsuite manually, I have a failure:

********* Start testing of ElfGNUSymbolVersioningTest *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20151207 (Red Hat 5.3.1-2))
PASS   : ElfGNUSymbolVersioningTest::initTestCase()
FAIL!  : ElfGNUSymbolVersioningTest::testSymbolVersioning() 'symDefIndex > 0' returned FALSE. ()
   Loc: [/home/developer/Projects/elf-dissector/tests/auto/elfgnusymbolversioningtest.cpp(60)]
PASS   : ElfGNUSymbolVersioningTest::testSymbolVersionDefinitions()
PASS   : ElfGNUSymbolVersioningTest::cleanupTestCase()
Totals: 3 passed, 1 failed, 0 skipped, 0 blacklisted
********* Finished testing of ElfGNUSymbolVersioningTest *********

But when i run the tests from within QtCreator, all the test suites pass! (incl. ElfGNUSymbolVersioningTest)
Is there a way to run the autotests via a make command?

Diff Detail

Repository
R739 ELF Dissector
Lint
Lint Skipped
Unit
Unit Tests Skipped
This comment was removed by christiangagneraud.
christiangagneraud retitled this revision from 0001-Fix-crashes-when-loading-a-core-dump.patch to Fix crashes when loading a core dump.Sep 25 2017, 3:41 AM
christiangagneraud edited the summary of this revision. (Show Details)
christiangagneraud edited the test plan for this revision. (Show Details)
  • you can use arc (the phabricator CLI) to submit patches
  • you can use ctest to run tests in CMake based applications (go to the build folder, run ctest)
lib/checks/dependenciescheck.cpp
50

the preferred style is

const auto *dynamicSection = fileSet->file(i)->dynamicSection();
if (!dynamicSection)
    continue;
foreach (const auto &needed, dynamicSection->neededLibraries()) {
christiangagneraud edited the summary of this revision. (Show Details)
christiangagneraud edited the test plan for this revision. (Show Details)

Fix coding style, as per review

Fix white-space issues

Fix white-space issues

vkrause accepted this revision.Sep 30 2017, 2:52 PM
This revision is now accepted and ready to land.Sep 30 2017, 2:52 PM
christiangagneraud marked an inline comment as done.Oct 5 2017, 7:52 AM

Hi,

I didn't take/want the kde dev account, and i was told to make sure that "one of the developers can push it for
you" == true.

Which is still not enough information: I would like to know if someone has actually pushed this review to mister master.

From here it looks accepted, but not merged.
git fetch github says: not here yet.

vkrause closed this revision.Oct 5 2017, 10:57 AM

done now, sorry, hadn't realized you didn't have push access