This is an archive of the discontinued LLVM Phabricator instance.

Update output for timestamp with llvm-readobj on XCOFF files.
ClosedPublic

Authored by stephenpeckham on Feb 17 2023, 10:11 AM.

Details

Summary

The llvm-readobj prints extra characters for the timestamp when --file-headers is used with an XCOFF file. This change updates the format string used to print the time. In addition, the timestamp is printed in the local timezone, and a thread-safe call is used to convert the time.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2023, 10:11 AM
stephenpeckham requested review of this revision.Feb 17 2023, 10:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2023, 10:11 AM
jhenderson requested changes to this revision.Feb 20 2023, 12:33 AM
jhenderson added inline comments.
llvm/tools/llvm-readobj/XCOFFDumper.cpp
105

This is failing to build on Windows.

This revision now requires changes to proceed.Feb 20 2023, 12:33 AM
stephenpeckham updated this revision to Diff 501128.EditedFeb 28 2023, 7:24 AM
stephenpeckham removed rG LLVM Github Monorepo as the repository for this revision.

This update backs out the changes except for removing the extra characters. WIthout a Windows system, the changes to use localtime_r() and update the test case are not practical. It would be nice to have a common function to print timestamps for all object file formats.

stephenpeckham marked an inline comment as done.Feb 28 2023, 7:25 AM
jhenderson accepted this revision.Mar 1 2023, 12:01 AM
jhenderson added a subscriber: Esme.

LGTMing this, to clear my objection, but I'm not a user of XCOFF, so I'd like one of the developers who regularly works on this to take a look too before this lands.

@Esme/@DiggerLin, any comments from either of you two?

@stephenpeckham, don't forget to update your commit message if you're dropping the localtime_r stuff.

This revision is now accepted and ready to land.Mar 1 2023, 12:01 AM