Opened 3 years ago
Closed 3 years ago
#13090 closed defect (fixed)
SYMBIAN: Build Failure Due to Certain Array Declaration Formats
Reported by: | fedor4ever | Owned by: | fedor4ever |
---|---|---|---|
Priority: | normal | Component: | Engine: Ultima |
Version: | Keywords: | Symbian, build error | |
Cc: | Game: | Ultima IV: Quest of the Avatar |
Description (last modified by )
Fixed by lephilousophe commit. Ultima 4&6 works! Thanks!
I look for initialization pointer to member and found nothing. This is bad code that violate C++ standart if there no initialization - https://isocpp.org/wiki/faq/pointers-to-members
I got error while building 2.5.0 release for Symbian:
elf2e32 : Error: E1066: Image failed validation line 427 make[1]: *** [\Symbian\S60_5th_Edition_SDK_v1.0\epoc32\release\gcce\urel\Neverhoode.exe] Error 1 make: *** [TARGET] Error 2
I dig more deeper and found in scummvm\engines\ultima\ultima4\game\item.cpp struct array declaration ItemLocation Items::ITEMS[N_ITEMS]. Commentin it's body solve error.
After commented body I inserting several zero initialazers:
{ nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, 0, 0 }
Build success.
Uncomment one initialazer - build success.
Uncomment another initialazer - build error.
Inserting function pointer in several zero initialazers trigger build error too.
scummvm/engines/ultima/nuvie/usecode/u6_object_types.h has similar struct array which cause same error.
scummvm/engines/ultima/nuvie/keybinding/keys.cpp - has Action NuvieActions[] with normal functions pointers - ScummVM builds and runs.
Change History (21)
comment:1 by , 3 years ago
Owner: | set to |
---|---|
Resolution: | → invalid |
Status: | new → closed |
comment:2 by , 3 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
I encounter a problem year ago - ScummVM can't start if Ultima engine built-in. I first encounter that while building release ScummVM 2.2.0. It was very hard to track down source error. So I cut down Ultima 4&6 support from Ultima engine in 2.2.0 release.
At this summer while building ScummVM I encounter such error. I reduce Ultima 4 subengine to it's detection table and entry point. Build comes flawless. I start enable warious parts while build was successful. After error happen scummvm\engines\ultima\ultima4\game\item.cpp was changed and error gone. I continue engine integration while all file compiled. Errors no more.
Then I reset my build environment except that change in scummvm\engines\ultima\ultima4\game\item.cpp. Build was successful and I saw Ultima 4 intro with music. I understand in that state game unplayable. I hope engine author plannig rewrite some engine parts.
About error message. I provide it. It's impossible to install several ScummVM.exe on one Symbian device. So Neverhoode comes. elf2e32 - tool for converting elf file to E32 aka EPOC32 aka Symbian binary. What means "line 427"? These idiots use that nonsense instead normal error print. I don't know what it means. That tool checks generated Symbian binary for correctness and print such nonsence.
comment:3 by , 3 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:4 by , 3 years ago
Description: | modified (diff) |
---|---|
Resolution: | invalid |
Status: | closed → new |
comment:5 by , 3 years ago
Description: | modified (diff) |
---|
comment:6 by , 3 years ago
I see that elf2e32 has been patched here: https://github.com/fedor4ever/elf2e32
That validation error is emitted from only two places in the code:
https://github.com/fedor4ever/elf2e32/blob/master/source/e32imagefile.cpp#L1095
https://github.com/fedor4ever/elf2e32/blob/master/source/e32imagefile.cpp#L1098
The iManager->E32ImageOutput() call will return "the name of the output E32 image output if provided as input through --output or nullptr":
https://github.com/fedor4ever/elf2e32/blob/master/source/parametermanager.cpp#L831
However, the Elf2e32Error::Elf2e32Error() constructor call takes the second parameter as a string and calls .c_str() on it with no check that it is not a nullptr:
https://github.com/fedor4ever/elf2e32/blob/master/source/errorhandler.cpp#L97
This causes the weird message of "line 427" above.
I think this can be fixed by changing https://github.com/fedor4ever/elf2e32/blob/master/source/errorhandler.cpp#L99 from:
Message::GetInstance()->ReportMessage(ERROR, iMessageIndex, aName.c_str());
to:
Message::GetInstance()->ReportMessage(ERROR, iMessageIndex, aName ? aName.c_str() : "");
And similarly for https://github.com/fedor4ever/elf2e32/blob/master/source/errorhandler.cpp#L104 ...
That will not fix this problem of compilation, but it will make the elf2e32 tool output more sane and reliable.
comment:7 by , 3 years ago
fedor4ever: While the code in Ultima4 seems to work fine and compile without warnings in my Linux desktop GCC, it is a bit "corner-case" and thus may provoke issues in older compilers.
I think there is probably an issue with the ARM GCC for Embedded Compiler as the elf2e32 tool seems to indicating that the elf file is not valid. You may want to compare the arm-objdump output from a "good" and "bad" file to work out what elf2e32 is choking on.
Are you using the original GCC from the SDK i.e. released around 2009? I assume not as this would not likely support C++-11 and I can see you have scripts to build a bleeding edge / newer GCC toolchain here:
https://github.com/fedor4ever/GCC4Symbian
As sev- indicates this is likely a toolchain issue. Assuming the "bad" elf is a valid format, this might be an issue with elf2e32 parsing the elf in some corner case... but hard to debug remotely.
comment:8 by , 3 years ago
Summary: | Build failure due ItemLocation array declaration → SYMBIAN: Build Failure Due to Certain Array Declaration Formats |
---|
comment:9 by , 3 years ago
Thans for pointing. I fixed many critical error but due bad code design started from scratch as elf2e32_next. It still incomplete.
There relocation alignment failure. I go deeper to https://github.com/fedor4ever/elf2e32/blob/ecfc39742dce03f3a5f4d83db18e8c1f395f3c47/source/portable.cpp#L427. I add printf after that line
if(offset&pointerAlignMask) {
printf("not aligned correctly");
RETURN_FAILURE(KErrCorrupt); } not aligned correctly
And see "not aligned correctly".
I use GCC 5.4.0, 5.5.0, 7.1.0, 11.2.0 with vary binutils. Currently use 11.2.0 with binutils 2.35. I have others too.
I can upload resulted elf file with required parameters for elf2e32.
I try co compile ultima\game\item.cpp with gcc 5.4.0 and link with rest.
comment:10 by , 3 years ago
I simple remove item.cpp from compiled object files cache and rebuild Ultima engine. Then try to build ScummVM. Used Gcc 5.4.0 with binutils 2.28. Same error.
Maybe there some compiler/linker option exist for alignment?
comment:11 by , 3 years ago
Thanks for:
aName ? aName.c_str() : ""
But tool checks these parameters before any job and throw exception. No needed any checks after :)
comment:12 by , 3 years ago
https://github.com/fedor4ever/elf2e32/blob/ecfc39742dce03f3a5f4d83db18e8c1f395f3c47/source/portable.cpp#L357 shows that ARM code here needs alignment to 4 bytes in the relocation addresses of the sections.
Try "readelf -a <name of elf>" or "<arm-eabi->objdump -r <name of elf>" to see what relocations are present and make sure the addresses are divisible by 4.
That will indicate what section is causing the issue.
I suggest you also read https://developer.arm.com/documentation/dui0474/f/image-structure-and-generation/section-alignment-with-the-linker ...
comment:13 by , 3 years ago
"objdump -r" prints elf type without any relocations.
I redirect readelf output to file. It has 23 megabytes. What data look for?
I see readelf truncated mangled functions name if it necessary.
comment:14 by , 3 years ago
Asked GCC devs for help. They needed small testcase. It was not fun due several god objects crossing entire engine.
comment:16 by , 3 years ago
Fixed some issues with GCC and C++ 11.
https://github.com/scummvm/scummvm/pull/3656
https://github.com/scummvm/scummvm/pull/3659
https://github.com/scummvm/scummvm/pull/3660
comment:17 by , 3 years ago
Got answer from GCC devs. This caused by packing struct. Answer - https://gcc.gnu.org/pipermail/gcc-help/2022-January/141141.html
comment:18 by , 3 years ago
@fedor4ever: Okaayy... So is this bug invalid i.e. it is actually a problem with the GCC toolchain for Symbian, or is there a fix required to the ScummVM source code?
As I said before "While the code in Ultima4 seems to work fine and compile without warnings in my Linux desktop GCC, it is a bit "corner-case" and thus may provoke issues in older compilers.", is this a compiler or code issue?
comment:19 by , 3 years ago
As said in GCC email packed Item struct has 41 byte size. Compiler generates right code and Symbian toolchain is fine too. GCC padding holes between word unaligned members if packing removed.
That code broken for all ARM 32 bit before ARMv7. These cpu lacking unaligned data access.
More about unaligned access - https://github.com/ARM-software/u-boot/blob/402465214395ed26d6fa72d9b6097c7adbf6a966/doc/README.unaligned-memory-access.txt#L46
I play little with small test and found two solution:
- Do not use packing at all. I hope this doesn't break engines or cause big memory usage.
- Add compile-time check and fix them. For example making last member in ItemLocation 4 byte making struct size 44 bytes. Buid successful.
comment:20 by , 3 years ago
This has already been fixed in master branch for almost two months.
https://github.com/scummvm/scummvm/commit/7003cf793f2ddc5b773293088a2b5e485bc8c105
You just have to backport this commit.
comment:21 by , 3 years ago
Description: | modified (diff) |
---|---|
Owner: | changed from | to
Resolution: | → fixed |
Status: | new → closed |
You are breaking the engine by commenting out the initalizer list. It is not like the true solution.
Moreover, you do not even provide the actual error message in your bugreport. What is "line 427"? What is the validation? What is Neverhoode.exe? You clearly screwed up your building environment.
Closing this as invalid.