Page MenuHomePhabricator

[AIX] Add improved interface for retrieving load module paths
Needs RevisionPublic

Authored by Jonathan.Crowther on Jan 14 2020, 2:37 PM.

Details

Summary

Implemented (for AIX) a function to take any function and return the path of the load module it is located in. Used this function to implement getMainExecutable for AIX (instead of incorrectly attempting to use /proc) Additionally, fixed multiple locations where getMainExecutable was used incorrectly.

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 14 2020, 2:37 PM
llvm/include/llvm/Support/FileUtilities.h
44 ↗(On Diff #238095)

I'm not sure that this function should handle failure by returning an empty string. The error condition can be made explicit by making the return type of the function llvm::Expected<SmallString<128>>.

llvm/include/llvm/Support/FileUtilities.h
38 ↗(On Diff #238095)

This should be an internal function in the .cpp file implementing the following function. It should not be declared in an interface .h file.

42 ↗(On Diff #238095)

The description of FileUtilities.h does not seem to encompass what this function does. Perhaps SystemUtils.h works better? fyi, when posting patches to Phabricator, the "full context" (using diff -U <large number>) should be provided.

Provided that you have squashed all changes into the latest commit in your branch, git diff HEAD^ -U10000 should emit the context you need. Adjust the number of lines of context accordingly to fit the size of your files.

Addressed comments by moving getLoadModuleFilnameForFunction to llvm/lib/Support/SystemUtils.cpp

xingxue accepted this revision.Feb 13 2020, 12:53 PM

LGTM.

This revision is now accepted and ready to land.Feb 13 2020, 12:53 PM
hubert.reinterpretcast requested changes to this revision.Feb 13 2020, 1:11 PM
hubert.reinterpretcast added inline comments.
llvm/lib/Support/Unix/Path.inc
243

The "expected" here is not checked for doing the error handling (soft-fail in this case). It appears LLVM_ENABLE_ABI_BREAKING_CHECKS would help to catch these.

This revision now requires changes to proceed.Feb 13 2020, 1:11 PM
llvm/lib/Support/Unix/Path.inc
243