This is an archive of the discontinued LLVM Phabricator instance.

[Object][COFF] Add table ptr checks but don't hard-error
AbandonedPublic

Authored by alvinhochun on Jun 1 2022, 8:41 AM.

Details

Reviewers
mstorsjo
rnk
Summary

When loading split debug files for PE/COFF executables (produced with
objcopy --only-keep-debug), the tables or directories in such files
may not exist. COFFObjectFile shall check the pointers to make sure they
are not out of bounds. One such check already exists for
initImportTablePtr. However, this check forces an error and prevents
the object file from being loaded, which means the file could not be
used as debug info in LLDB. To fix this, it is sufficient to just hide
the error and pretend the import table does not exist for the object
file.

In the same way, other functions which load the pointers for other
tables should also contain similar pointer checks, but none of these
functions actually have the check in place. This commit adds these
missing checks.

Fixes https://github.com/mstorsjo/llvm-mingw/issues/284

Diff Detail

Event Timeline

alvinhochun created this revision.Jun 1 2022, 8:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 8:41 AM
alvinhochun published this revision for review.Jun 1 2022, 9:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 9:32 AM

This needs a testcase - maybe one for llvm-readobj or llvm-objdump, to make sure it doesn't error out on such an object file. I believe you're familiar with the obj2yaml/yaml2obj kind of test object files used (based on some other patch of yours) - I guess it should be kinda doable to pick an existing minimal-ish yaml for e.g. an executable, and modify it to reach the state where the import tables point out of bounds, to trigger the issue (producing a case that didn't use to work before but now works).

llvm/lib/Object/COFFObjectFile.cpp
608

The // ! comments should be left out when upstreaming the patch.

alvinhochun planned changes to this revision.Jun 2 2022, 1:48 AM