Opened 6 months ago

Closed 4 months 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 fedor4ever)

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 sev-, 6 months ago

Owner: set to sev-
Resolution: invalid
Status: newclosed

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.

comment:2 by fedor4ever, 6 months ago

Resolution: invalid
Status: closednew

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.

Last edited 6 months ago by fedor4ever (previous) (diff)

comment:3 by sev-, 6 months ago

Resolution: invalid
Status: newclosed

comment:4 by fedor4ever, 6 months ago

Description: modified (diff)
Resolution: invalid
Status: closednew

comment:5 by fedor4ever, 6 months ago

Description: modified (diff)

comment:6 by digitall, 5 months 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 digitall, 5 months 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 digitall, 5 months ago

Summary: Build failure due ItemLocation array declarationSYMBIAN: Build Failure Due to Certain Array Declaration Formats

comment:9 by fedor4ever, 5 months 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 fedor4ever, 5 months 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?

Last edited 5 months ago by fedor4ever (previous) (diff)

comment:11 by fedor4ever, 5 months 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 digitall, 5 months 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 fedor4ever, 5 months 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 fedor4ever, 5 months ago

Asked GCC devs for help. They needed small testcase. It was not fun due several god objects crossing entire engine.

comment:15 by fedor4ever, 5 months ago

Sent small testcase to gcc devs. Waiting for response.

comment:17 by fedor4ever, 4 months 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 digitall, 4 months 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 fedor4ever, 4 months 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:

  1. Do not use packing at all. I hope this doesn't break engines or cause big memory usage.
  2. Add compile-time check and fix them. For example making last member in ItemLocation 4 byte making struct size 44 bytes. Buid successful.
Last edited 4 months ago by fedor4ever (previous) (diff)

comment:20 by lephilousophe, 4 months 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 fedor4ever, 4 months ago

Description: modified (diff)
Owner: changed from sev- to fedor4ever
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.