BaseTools: Simplify Resource-file rule and fix GCC toolchain command#79
BaseTools: Simplify Resource-file rule and fix GCC toolchain command#79martinezjavier wants to merge 1 commit intomicrosoft:release/202008from
Conversation
This rule contains several issues that causes the VERSIONINFO resource to not be embedded into the compiled binaries, when using the GCC toolchain. The problems with the rule when using the GCC toolchain are the following: * The windres command executed does not contain an --output-format option, so the tool tries to guess the format and just generates a .rc file. * The OutputFile.GCC is set to $(MODULE_NAME)Resource.rc, instead of an binary object file that could be included when building the ELF objects. * The resource is never added to a binary file so is not included in the final PE32+ executable binary. One solution may be to make windres generate a COFF object file and use the objcopy tool to convert the COFF object file to an ELF object file, that way the resource would be picked when building the ELF executable. That is what the commented out commands in the rule attempt to do, but it won't work since the ELF binary built is a Position Independent Executable (PIE). The linker then would complain that relocation symbols included in the ELF object file created by objcopy cannot be used when making a PIE. Another option is to convert the .rc source resource into a .res binary format and then embed that in as a section using objcopy --add-section. And there is already a Hii-Binary-Package.UEFI_HII rule that does exactly that, it gets hpk files as input and adds the content of those to a .rsrc section in the binaries. Use that rule instead to avoid duplicated logic. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
| <Command.MSFT, Command.INTEL, Command.CLANGPDB> | ||
| "$(RC)" /Fo${dst} ${src} | ||
| <OutputFile.MSFT, OutputFile.INTEL, OutputFile.GCC, OutputFile.CLANGPDB> | ||
| $(OUTPUT_DIR)(+)$(MODULE_NAME)Resource.hpk |
There was a problem hiding this comment.
If we output HPK will this mean that version resource and hii resource are exclusive since the output file of HPK rule above is the same? I like the reuse of HII but we should support a module that uses both.
There was a problem hiding this comment.
If we output HPK will this mean that version resource and hii resource are exclusive since the output file of HPK rule
No, because the content of all the different .hpk files will be added to the $(MODULE_NAME)hii.rc.
In other words, the $(HII_BINARY_PACKAGES) variable in the command of the Hii-Binary-Package.UEFI_HII rule gets expanded to all the .hpk files, so the final .rsrc section in the PE32+ binary will contain the data of all the hpk files.
For example, on my test machine this is the actual commands executed by the Hii-Binary-Package.UEFI_HII rule:
INFO - "GenFw" -o /home/javier/devel/mu_basecore/Build/MdeModule/DEBUG_GCC5/X64/MdeModulePkg/Application/HelloWorld/HelloWorld/OUTPUT/HelloWorldhii.rc -g 6987936E-ED34-44db-AE97-1FA5E4ED2116 --hiibinpackage /home/javier/devel/mu_basecore/Build/MdeModule/DEBUG_GCC5/X64/MdeModulePkg/Application/HelloWorld/HelloWorld/OUTPUT/HelloWorldResource.hpk /home/javier/devel/mu_basecore/Build/MdeModule/DEBUG_GCC5/X64/MdeModulePkg/Application/HelloWorld/HelloWorld/OUTPUT/HelloWorldStrDefs.hpk
INFO - "objcopy" -I binary -O elf64-x86-64 -B i386 --rename-section .data=.hii /home/javier/devel/mu_basecore/Build/MdeModule/DEBUG_GCC5/X64/MdeModulePkg/Application/HelloWorld/HelloWorld/OUTPUT/HelloWorldhii.rc /home/javier/devel/mu_basecore/Build/MdeModule/DEBUG_GCC5/X64/MdeModulePkg/Application/HelloWorld/HelloWorld/OUTPUT/HelloWorldhii.lib
There was a problem hiding this comment.
@spbrogan and thanks a lot for your feedback and review!
There was a problem hiding this comment.
so they all get merged into the one .rc?
Have you tried running the version_info tool on the binary and see the results match expected? You will need at least version 0.14.0 of edk2-pytool-extensions
versioninfo_tool -d {/path/to/app.efi} {/path/to/output.JSON}
https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_versioninfo.md
There was a problem hiding this comment.
so they all get merged into the one .rc?
Yes, my understanding is that this is already the case for other build rules that output a .hpk file.
Have you tried running the version_info tool on the binary and see the results match expected? You will need at least version 0.14.0 of edk2-pytool-extensions
I was just testing with strings -e l and objdump -s -j .rsrc but I've tried now with that script and it fails...
$ strings -e l ./Build/MdeModule/DEBUG_GCC5/X64/HelloWorld.efi
UEFI Hello World!
VS_VERSION_INFO
VS_VERSION_INFO
StringFileInfo
040904b0
CompanyName
Example Company Name
OriginalFilename
HelloWorld.efi
VarFileInfo
Translation
English
.TH HelloWorld 0 "Displays a "UEFI Hello World!" string."
.SH NAME
HelloWorld application.
$ versioninfo_tool -d ./Build/MdeModule/DEBUG_GCC5/X64/HelloWorld.efi test.json
Could not find VS_FIXEDFILEINFO.
Traceback (most recent call last):
File "/usr/local/bin/versioninfo_tool", line 33, in <module>
sys.exit(load_entry_point('edk2-pytool-extensions==0.14.0', 'console_scripts', 'versioninfo_tool')())
File "/usr/local/lib/python3.9/site-packages/edk2toolext/versioninfo/versioninfo_tool.py", line 105, in main
if not decode_version_info_dump_json(args.input_file, args.output_file):
File "/usr/local/lib/python3.9/site-packages/edk2toolext/versioninfo/versioninfo_tool.py", line 74, in decode_version_info_dump_json
version_info = decode_version_info(input_file)
File "/usr/local/lib/python3.9/site-packages/edk2toolext/versioninfo/versioninfo_tool.py", line 66, in decode_version_info
return pe.get_version_dict()
File "/usr/local/lib/python3.9/site-packages/edk2toolext/versioninfo/versioninfo_helper.py", line 395, in get_version_dict
for fileinfo in self._pe.FileInfo:
AttributeError: 'PE' object has no attribute 'FileInfo'
To make sure that it wasn't a problem with the script, I just tried to load the binary and print the PE class VS_FIXEDFILEINFO attribute but that doesn't work either:
>>> import pefile
>>> path = "./Build/MdeModule/DEBUG_GCC5/X64/HelloWorld.efi"
>>> pe = pefile.PE(path)
>>> print(pe.VS_FIXEDFILEINFO)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'PE' object has no attribute 'VS_FIXEDFILEINFO'
>>>
I see that the same problem happens if I just take a binary that doesn't have a .rsrc section and attempt to embed a binary .res file with objcopy HelloWorld.efi --add-section .rsrc=HelloWorldResource.rc.
I will take a look tomorrow to understand what's missing or wrong with this approach.
|
Comparing the PE/COFF binaries I notice that the problem is that the resource directory doesn't have the correct structure. On a binary where the version information is reported, the resource directory looks like the following: while my I wrongly assumed that I believe that I'll close this since the approach is not correct and seems to be more complicated than I thought. Sorry @spbrogan for the noise and thanks again for your review. |
|
@martinezjavier Thanks for the PR. I think more has definitely been learned and the information in the commit and PR looks helpful. I agree that there is more investigation needed. Anyway I appreciate the information. |
2 participants
This rule contains several issues that causes the VERSIONINFO resource to
not be embedded into the compiled binaries, when using the GCC toolchain.
The problems with the rule when using the GCC toolchain are the following:
The windres command executed does not contain an --output-format option,
so the tool tries to guess the format and just generates a .rc file.
The OutputFile.GCC is set to $(MODULE_NAME)Resource.rc, instead of an
binary object file that could be included when building the ELF objects.
The resource is never added to a binary file so is not included in the
final PE32+ executable binary.
One solution may be to make windres generate a COFF object file and use
the objcopy tool to convert the COFF object file to an ELF object file,
that way the resource would be picked when building the ELF executable.
That is what the commented out commands in the rule attempt to do, but it
won't work since the ELF binary built is a Position Independent Executable
(PIE). The linker then would complain that relocation symbols included in
the ELF object file created by objcopy cannot be used when making a PIE.
Another option is to convert the .rc source resource into a .res binary
format and then embed that in as a section using objcopy --add-section.
And there is already a Hii-Binary-Package.UEFI_HII rule that does exactly
that, it gets hpk files as input and adds the content of those to a .rsrc
section in the binaries. Use that rule instead to avoid duplicated logic.
Signed-off-by: Javier Martinez Canillas javierm@redhat.com