Adding functionality to readobj to display resource section data. This is necessary in order to test the cvtres tool when it is added.
Details
Diff Detail
- Build Status
Buildable 5885 Build 5885: arc lint + arc unit
Event Timeline
I think this is a bit too small of a change. I know I said to do it incrementally and not as one big patch, but it seems like we should be able to do something meaningful. For example, there's a .rsrc$1 and .rsrc$2, perhaps you could display some metadata about them and dump the raw bytes of the sections without parsing the tree structure and individual resources.
Since this is printing something real now, can we add a FileCheck test for it?
llvm/include/llvm/Object/COFF.h | ||
---|---|---|
1064–1066 | Seems like either all fields should be initialized inline, or none should. Is there a reason only OwningObject gets the special treatment? | |
llvm/lib/Object/COFFObjectFile.cpp | ||
454 | delete this debug statement as well as the subsequent ones as well. | |
llvm/tools/llvm-readobj/COFFDumper.cpp | ||
1536–1537 | This will miss a couple of cases.
D:\src\llvmbuild\ninja>bin\llvm-objdump -section-headers bin\clang.exe bin\clang.exe: file format COFF-i386 Sections: Idx Name Size Address Type ... 6 .rsrc 0000075c 0000000006bb2000 DATA ...
| |
1540–1541 | Move these two lines up out of the if statement. | |
1544–1549 | It looks like this is a time_t? We have some functions so you don't have to reproduce this formatting code every time. #include "llvm/Support/Chrono.h" ... auto Time = sys::toTimePoint(TimeDateStamp); SmallString<20> FormattedTime; raw_svector_ostream OS(FormattedTime); OS << Time; W.printHex("Time/Date Stamp", FormattedTime, TimeDateStamp); | |
1553–1555 | Then delete this branch and call printBinaryBlock below. |
llvm/tools/llvm-readobj/COFFDumper.cpp | ||
---|---|---|
1542–1543 | I don't see why you need this ResourceDirTableRef class. Since you are accessing section data by casting it to coff_resource_dir_table *, you can just access members directly. I mean, you could do something like this. auto *Ent = reinterpret_cast<const coff_resource_dir_table *>(Ref.data())); ... time_t TDS = time_t(Ent->TimeDateStamp); This is I think the usual way of accessing COFF section data. |
llvm/include/llvm/Object/COFF.h | ||
---|---|---|
1064–1066 | This pattern is followed for all other section wrapper classes in this file. I don't understand the inherent benefit of doing this but I see no reason to break the pattern either. | |
llvm/tools/llvm-readobj/COFFDumper.cpp | ||
1536–1537 | Ah okay...I was thinking for now we can just build the feature around reading .o's and not .exe's because that's what we want to test, but if it's not much more effort we might as well do both. | |
1542–1543 | True, however in the coming patches I think the ResourceDirTableRef class will be extended to do many more complicated things, like traversing the tree structure and returning certain resource data. I think it will be good to encapsulate this. | |
1544–1549 | Done, however it looks like using the TimePoint library gives times based on 1969-12-31 instead of 1970-1-1, is this expected behavior? |
llvm/include/llvm/Object/COFF.h | ||
---|---|---|
1064–1066 | Adding a wrapper that you don't need and even understand existential reason is not a good idea. You can add it later if you need it, but at the moment this new code just complicates the code unnecessarily. |
llvm/tools/llvm-readobj/COFFDumper.cpp | ||
---|---|---|
1544–1549 | Hmm, I find this a bit surprising. Looking at the code, it calls std::chrono::system_clock::from_time_t which takes a std::time_t. cppreference says that it's not technically defined, but almost always uses the unix epoch. I would expect that your implementation is also using the unix epoch. So maybe it's a problem in the printing code? Can you paste the exact formatted string both with the original code and the operator<< code? |
I seems like a timezone issue. We are in PST, and PST is UTC-0800, which describes why you got 1969-12-31 16:00:00 instead of 1970-00-00 00:00:00. You want to print it out in UTC.
Seems like either all fields should be initialized inline, or none should. Is there a reason only OwningObject gets the special treatment?